11 Commits

Author SHA1 Message Date
Andrey Antukh
9abcfebf60
✨ Run binfile import and export as durable jobs (#12056)
* ✨ Run binfile import and export as durable jobs

Import and export of `.penpot` files become durable jobs: two commands
create them and answer at once with a job id the caller follows, while
a worker of the `binfile` queue does the work. Each job keeps its own
ledger (owner, params frozen at creation, result, progress history and
an expiry), and the legacy commands and their SSE keep working.

Why: both operations ran inside the request. A large import held it
open for minutes, progress only existed while the caller kept the
stream open, and a client that went away left work nobody recorded,
retried or could cancel. The unified `job` substrate already dispatched,
leased, retried and logged events, but it had no user-facing ledger.

How:
- The substrate gains the pieces a user-facing job needs: optional
  job-def metadata (family, resource-role), an expiry on submit, the
  owner in the runner context, and job resources in their own bucket,
  written with the profile of the job and never deduplicated.
- One generic command per family: the caller names the job and sends its
  business params, the registry checks the family, the job-def decodes
  and validates, and permission and quota are checked first.
- Progress taps of the core become milestones of the job: a shared stage
  vocabulary with self-contained counters, throttled heartbeats, and an
  interrupt that stops the run inside its own transaction on cancel.
- Cancel reaches pending and running jobs from both modals, and closing
  a modal mid-run cancels the job it was following.
- The export re-checks the read permission at run time and answers with
  the package descriptor; the import re-checks the edition permission,
  runs the core in a transaction so a cancellation rolls it back, and
  releases the consumed package and its temporary copy.

AI-assisted-by: deepseek-v4.1-flash

* ♻️ Handle externally-cancelled binfile jobs end to end

The `cancelled` outcome of a job nobody followed no longer hangs its
screens: the data layers emit an explicit terminal message for it.

- The export emits `{:cancelled true}` per file and the modal paints a
  neutral cancelled row instead of success; a message without an
  artifact never reaches the download branch.
- The import closes every entry with a dedicated cancelled message
  through the existing error rendering, so the wizard completes.
- Both flows are pinned by data-layer tests, including the red state
  they replace.

Also in this change: the progress-contract comment of the management
API, early business-params validation in create-import-job (before
assembling the upload), a `release-resource` helper without fabricated
job ids, a note on the heartbeat write cost, and a memory cleanup.

AI-assisted-by: muse-spark-1.3-contributor

* ♻️ Pre-translate the cancelled import message

The cancelled entry of the import wizard carried its message as a key
for the UI to translate, which needed a rehash marker comment. Like
its neighboring errors, it now translates eagerly in the data layer:
the literal call keeps rehash clean with no marker, and the tests
assert through the same call instead of the raw key.

AI-assisted-by: muse-spark-1.3-contributor

* ♻️ Move cancelled strings under generic labels keys

The dashboard-specific cancelled key never shipped: before use it moves
under `labels.*` next to the generic strings both flows already use,
with Spanish translations following the established wording.

AI-assisted-by: muse-spark-1.3-contributor

* ✨ Add minor improvements

* ♻️ Rename export job command to create-export-binfile-job

Export now has one command per job type, the shape the :export-assets
command needs: create-export-binfile-job freezes exactly the
:export-binfile job, so :name no longer travels in the body and only
:params does. Import keeps the generic envelope.

submit-job reads the queue from the job-def metadata ::jobs/queue-name
(falling back to :binfile), read by the command and never by the
substrate, so a job-def can route its work to another queue without a
change in the command.

The rename reaches the frontend and the tests: the binfile export is
asked for under its own name, and the exporter service keeps
create-export-job.

AI-assisted-by: deepseek-v4.1-flash

* ✨ Polish the import and export status rows

The status row of a file already carries what the file is doing, so the
generic "Uploading file…"/"Downloading file…" lines are gone: the
import wizard drops its own (the export one went with the jobs
refactor), along with the now-unused style.

In the export, every file is queued up front, even the ones whose job
is created only after the previous one is over, and a file stays queued
until its first milestone: a worker picking the job up no longer ends
the queue. The queued text is a step smaller than the milestone.

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Show the cancelled export and fix the import error

The cancelled export was unreachable: mark-file-cancelled sets :loading
false and the label lived inside the loading branch, so a cancelled
file only painted a neutral notification. The terminal state now
carries the label, and the export flags lose the `?` suffix
(predicates keep it; a data prop is not a question).

The import wizard rendered (:error entry) through tr, but every
producer writes user-facing text (a hint or a translated label), never
a key: it renders the text as-is now, and the dynamic-key ignores and
the stale comment are gone.

The total-text-limit comment no longer references the 200MiB it
replaced; the value is 500MiB in develop already.

AI-assisted-by: deepseek-v4.1-flash

* ♻️ Reflect the real job params in the creation commands

The creation commands declared their params as an open empty map, so the
RPC contract and the generated docs said nothing about them. Each command
now imports the schema from the job namespace
(`export-binfile/schema:params`, `import-binfile/schema:create-params`),
and the RPC validates the business params before the command runs. The
job-def still owns the params and re-validates the assembled ones at run
time. The commands no longer decode the params again: the RPC did it.

Import joins export as one command per job type: `create-import-job`
becomes `create-import-binfile-job`, its body no longer carries `:name`
and the job-def is resolved by name like export's. Its creation schema
keeps the version optional, because the command reads it from the package
header when the caller does not say it, and fills the manifest metadata
kept for audit.

A version out of range is now refused by the schema (`:params-validation`)
instead of the command's own `:unsupported-version` check, which is gone.

AI-assisted-by: deepseek-v4.1-flash

* 🐛 Stop the test system from running job runners

The test system is main/system-config plus main/worker-config with a
few components dissoc'ed. The runners of default, webhook and cron were
listed by hand; the binfile runner added with the durable binfile jobs
was not, so a real penpot/job-runner/binfile/0 thread ran during the
whole suite.

That runner races the tests that drive the loop by hand (run-batch plus
run-worker-loop): it takes the job the test just dispatched and
completes it, and the test then reads a state it did not produce. That
is the reported flake of binfile-jobs-integration-test, whose middle
assertion waits for a runner of another queue to not take the job. It
only reproduced in CI: in the devenv PENPOT_TENANT makes the tenant
baked into main/worker-config differ from the one the test builds, so
the two used different queue keys locally.

Drop every [<profile> :app.worker/runner] by key shape instead of
listing the known queues, so a new queue cannot bring the race back.

AI-assisted-by: deepseek-v4.1-flash

* ✨ Upload the package one chunk at a time and as a percentage

The upload milestone counts chunks, not units of work, so the row wrote
"Uploading the package 3/9" over a file the user had uploaded once: it
read as if nine packages were on their way. Write the stage as a
percentage of the file instead ("Uploading file… (33%)"), and make the
English text of the stage say file, not package.

The chunks also go one at a time now (concat-all instead of merge-all
with two in flight), so the percentage moves in a straight line instead
of jumping. upload-blob-chunked is the generic uploader, so media and
font uploads become sequential too.

AI-assisted-by: deepseek-v4.1-flash

* 🌐 Complete the Spanish catalogue

es.po was 25 keys short of en.po: the 15 that the durable binfile jobs
introduced (14 jobs.progress.stage.* and labels.queued) plus 10 gaps
that predate them (errors.connection-error, errors.save-retrying, the
find shortcuts, workspace.header.retrying,
workspace.tokens.stroke-width and the mcp session labels).

sync -l es then sorts the entries, syncs the #: comments from en and
drops the 7 dashboard.import.progress.* keys en no longer has. The
msgid sets of en and es are now identical (0 missing, 0 extra).

AI-assisted-by: deepseek-v4.1-flash

* ♻️ Move the job status keys into the jobs namespace

The three status words the durable import and export rows render were
filed under labels.*, next to the generic labels bucket, while their
progress stages live under jobs.progress.stage.*: the strings of one
feature sat in two places.

Rename them so the whole job vocabulary is one namespace:

  labels.queued           -> jobs.queued
  labels.export-cancelled -> jobs.export-cancelled
  labels.import-cancelled -> jobs.import-cancelled

No other locale has these keys yet, so the rename costs no translation.
The convention (a key names the feature that renders it; labels.* holds
the generic reusable words) is written down in the frontend/translations
memory.

AI-assisted-by: deepseek-v4.1-flash

* 🔥 Remove the translation keys nothing references

Six keys of en.po (and of es.po, which now mirrors it) have no (tr ...)
call site left in frontend/src or common/src, and no literal reference
either:

  labels.uploading-file    a duplicate of jobs.progress.stage.upload
                           since the upload stage says "Uploading file…"
  labels.downloading-file  dead since the durable jobs replaced the old
                           import and export progress rows
  inspect.attributes.stroke.alignment.center
  inspect.attributes.stroke.alignment.inner
  inspect.attributes.stroke.alignment.outer
  workspace.toolbar.mcp-connect-here

The first two are leftovers of the durable binfile jobs, the other four
predate them. Both catalogues keep an identical msgid set afterwards.

AI-assisted-by: deepseek-v4.1-flash
2026-10-07 11:52:48 +02:00
Andrey Antukh
b5fbc4fd8c
✨ Add size limits to profile props and plugin registry (#11596)
* ✨ Add size limits to profile props and plugin registry

Bound the total serialized size of profile settings to 2 MiB
(:profile-props-max-size), checked on the merged result before
persisting, with a controlled :props-too-large error. Profiles
that already exceed the limit can still shrink but cannot grow.

Cap plugin registry entries in the shared schema (code 1 MiB, 50
plugins max, bounded name/host/description/icon) and restore rate
limiting on the plugin RPCs (profile-mutations bucket, one write
at a time per profile). The plugin manager now asks for
confirmation before removal and ignores repeated clicks while a
persist request is in flight.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Enforce plugin count cap, byte sizes and removal guard

Enforce the declared 50-plugin cap in add-profile-plugin with a
specific :too-many-plugins error (updates of existing entries
still pass); the cap lives in a shared max-plugins constant.

Measure profile props size in UTF-8 bytes instead of chars so
multibyte content cannot slip past the limit.

Cover install/remove persist logic with mocked-RPC frontend tests
(release semantics, in-flight dedupe, validation vs rollback
split) and add the missing boundary tests in common.

Expose the in-flight persist set from the plugin registry and
disable the remove button of entries being saved.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Fix rollback loops and restore paths in plugin registry

Restore the previous plugin version instead of dropping the entry
when a validation error rejects an update of an installed plugin.

Make compensating writes one-shot with terminal callbacks so a
persistent failure cannot ping-pong between install and remove.
Restores keep the original list position; the unused public
plugin-persisting? predicate is removed.

Pin count-before-size precedence with a dedicated test and fix
translation source refs to their canonical lines.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Guard notifications write and fix restore ordering

Route update-profile-notifications through check-props-size! so
oversized profiles cannot grow through that path; document the
exempt system writers. Remove the duplicated stale entries in
en.po, keeping the canonical translation refs.

Restore rejected plugin updates at their original list position
instead of leaving the optimistic move in place.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Skip no-op plugin removal and clarify size comments

Return early from remove-profile-plugin when the id is absent:
no wasted write, no size check, and no manufactured :plugins key
that could spuriously fail on oversized profiles.

Clarify that per-field string caps count chars while the byte
budget is enforced by profile-props-max-size.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

* 📎 Fix formatting in rlimit.edn for profile operations

Signed-off-by: Andrey Antukh <niwi@niwi.nz>

* 📎 Fix formatting of import-binfile/global entry

Signed-off-by: Andrey Antukh <niwi@niwi.nz>

* ♻️ Simplify props size check and tighten plugin entry caps

Measure props with transit bytes directly instead of the
PGobject string roundtrip.

Rename check-props-size! to check-props-size: single hard limit
on the merged props, no growth comparison, and return props so
writers thread the check into the update.

Move the 2 MiB default into default-props-max-size on the
profile namespace, still overridable with the optional
:profile-props-max-size config entry.

Tighten registry-entry :code and :icon to 500 chars: they hold
manifest paths, not content.

Closes #11592

AI-assisted-by: muse-spark-1.3-contributor

* 🐛 Fix compatibility problems

---------

Signed-off-by: Andrey Antukh <niwi@niwi.nz>
Co-authored-by: alonso.torres <alonso.torres@kaleidos.net>
2026-10-01 14:31:30 +02:00
Andrey Antukh
3033da4409
🐛 Add concurrency limit to import-binfile RPC handler (#11024)
Apply climit with 4 global permits and 1 per-profile permit (queue 2)
to prevent connection pool exhaustion from concurrent imports. Each
import holds a DB connection for its entire duration with idle
transaction timeout disabled, so unbounded concurrency could exhaust
the pool (default 60 connections).

AI-assisted-by: mimo-v2.5-pro
2026-08-17 11:08:07 +02:00
Andrey Antukh
4f7bb94bb1
🐛 Add size limit and rate limiting to send-user-feedback (#10979) (#10990)
Prevent email bombing attacks on the send-user-feedback endpoint by
limiting the error-report field to 1MiB and adding climit rate limits:
by-profile (1 permit, queue 3) and global (4 permits), configured in
climit.edn. Make the schema public so it can be exercised by tests,
and add schema validation tests covering the new size limit.

AI-assisted-by: qwen3.7-plus
2026-08-05 17:35:59 +02:00
Andrey Antukh
405a73e8ba
✨ Add climit impl and config for file snapshot methods (#9722)
* ✨ Add dedicated concurrency limit for restore-file-snapshot

This adds a dedicated climit configuration for the restore-file-snapshot
RPC method with :permits 1 per profile (plus queue of 2 and 60s timeout)
and a global limit of 3. Previously the method only used the generic
root/by-profile and root/global limits, allowing up to 7 concurrent
restore operations per profile which caused database row lock contention
on FOR UPDATE and connection pool exhaustion.

* ✨ Skip locking on restore! to avoid blocking other operations

Changes the row lock acquisition in restore! from a blocking FOR UPDATE
to FOR UPDATE SKIP LOCKED. If the file row is already locked by another
concurrent operation (e.g., another restore or an update-file), the query
returns no rows and the caller fails fast with a clear conflict error
instead of blocking indefinitely holding a database connection.

* ✨ Add queue and timeout limits to root/by-profile concurrency limit

Previously root/by-profile had no queue limit (unbounded Integer/MAX_VALUE)
and no timeout, allowing requests to pile up indefinitely behind a profile
whose permits were exhausted by long-running operations. This could lead
to memory pressure and cascading failures. Now limited to 30 queued
requests with a 30-second timeout so excess requests fail fast.

* ✨ Move backup snapshot creation outside restore transaction

The backup snapshot (fsnap/create!) is now created in its own short-lived
connection before the actual restore transaction begins. This ensures the
backup is persisted independently of the restore outcome and reduces the
restore transaction window.

The restore itself runs inside a db/tx-run! block with an optimistic
locking check: it reads the file with FOR UPDATE and compares its revn
against the value captured at backup time. If the file was edited
concurrently, the restore aborts with a conflict error to prevent data
loss.

Co-dependent with the SKIP LOCKED change in restore! — the FOR UPDATE
acquired here is in the same transaction as restore!, so the SKIP LOCKED
inside restore! correctly sees the row as unlocked (same transaction).

* ♻️ Remove unused private function get-minimal-file

The local get-minimal-file function in file_snapshots.clj is no longer
used since restore! switched to direct exec-one! with FOR UPDATE SKIP
LOCKED. The sql:get-minimal-file SQL constant is still used directly.

* ✨ Add minor improvements on db connection management

* ♻️ Refactor create-file-snapshot to use explicit transaction management

Remove automatic transaction wrapping (`::db/transaction true`) and
pass `cfg` through the call chain instead of destructured `conn`.
Wrap `fsnap/create!` in an explicit `db/tx-run!` for clearer
transaction boundaries.

Signed-off-by: Andrey Antukh <niwi@niwi.nz>

* ✨ Add dedicated concurrency limit for create-file-snapshot

This adds a dedicated climit configuration for the create-file-snapshot
RPC method with :permits 1 per profile (plus queue of 2 and 60s timeout)
and a global limit of 3. Previously the method only used the generic
root/by-profile and root/global limits, allowing up to 10 concurrent
snapshot creation operations per profile which could cause database
contention and connection pool exhaustion.

Signed-off-by: Andrey Antukh <niwi@niwi.nz>

---------

Signed-off-by: Andrey Antukh <niwi@niwi.nz>
2026-05-19 14:30:44 +02:00
Andrey Antukh
a5c6d78ee5 ♻️ Fix some fundamental bugs on climit module
The climit previously of this commit is heavily used inside a
transactions, so in heavy contention operation such that file thumbnail
creation can cause a db pool exhaust.

This commit fixes this issue setting up a better resource limiting
mechanism that works outside the transactions so, contention will
no longer hold an open connection/transaction.

It also adds general improvement to the traceability to the climit
mechanism: it now properly logs the profile-id that is currently
cause some contention on specific resources.

It also add a general/root climit that is applied to all requests
so if someone start making abussive requests, we can clearly detect
it.
2024-02-01 17:37:49 +01:00
Andrey Antukh
54341d5b22 ✨ Make the RPC climit subsystem more robust 2023-11-27 14:25:12 +01:00
Andrey Antukh
da68eae7c6 ✨ Add proper climit configuration for file-thumbnails 2023-11-08 17:11:42 +01:00
Andrey Antukh
aafbf6bc15 ♻️ Refactor cocurrency model on backend
Mainly the followin changes:

- Pass majority of code to the old and plain synchronous style
  and start using virtual threads for the RPC (and partially some
  HTTP server middlewares).
- Make some improvements on how CLIMIT is handled, simplifying code
- Improve considerably performance reducing the reflection and
  unnecesary funcion calls on the whole stack-trace of an RPC call.
- Improve efficiency reducing considerably the total threads number.
2023-03-14 12:30:27 +01:00
Andrey Antukh
7f589b09ca ♻️ Move audit http handler to RPC 2022-12-13 16:17:31 +01:00
Andrey Antukh
37ad04d2a6 🎉 Add robust concurrency limiter for RPC 2022-11-07 10:05:56 +01:00