Implement penpot_commit with revn tracking, conflict retry and commit preview #195

New issue

Closed

opened 2026-09-14 23:10:33 +00:00 by david · 1 comment

david commented

2026-09-14 23:10:33 +00:00

Owner

Summary

Implement penpot_commit, penpot_discard and penpot_status: apply the whole staged changeset as a single update-file call with correct revn handling, a preview of exactly what will change, and a post-write verification by re-reading the file.

Background

Depends on: #194

This is the only place the extension writes. Getting revn right here protects every later write path, because the semantics are counter-intuitive and were confirmed empirically on Penpot 2.17:

update-file body is { id, sessionId, revn, vern, changes }.
The response's revn is the pre-write revision, not the new one.
A lagged (lower) revn is accepted and applied — it is not an optimistic lock. Writing twice with the same stale revn applied both writes.
Only a revn greater than the stored one errors (revn-conflict); a mismatched vern raises vern-conflict.
The response's lagged array echoes other revisions and is not an acknowledgement of your own write. Never treat it as confirmation.
A rejected batch increments nothing and applies nothing (atomic).

Consequence: correctness depends on the extension reading revn immediately before writing and confirming success by re-reading the file, not by parsing the response.

sessionId is a per-session UUID the server uses to attribute changes; generate one per extension session.

Documentation Required

A separate process downloads these into the listed folders before this issue is implemented. Check the folders for the actual reference material before starting.

docs/reference/penpot-api/

<PENPOT_URL>/api/main/doc/openapi.json — the update-file command: body parameters, response fields (revn, vern, lagged) and the error types it can raise.
https://raw.githubusercontent.com/penpot/penpot/2.17.2/common/src/app/common/types/file.cljc — the file revision/version fields the commit reads and reports.
https://raw.githubusercontent.com/penpot/penpot/2.17.2/common/src/app/common/types/modifiers.cljc — the change-type/modifier union: what an update-file change list accepts, useful for validating a staged batch before sending.
https://help.penpot.app/technical-guide/integration/ — auth and command URL shape.
https://help.penpot.app/user-guide/ — expectations for what a designer sees after a change (used for the live validation wording).

docs/reference/pi-coding-agent/ and docs/reference/typebox/

https://pi.dev/docs/latest/extensions — tool registration and returning structured details.
https://github.com/sinclairzx81/typebox — boolean/string parameter schemas for a dry_run/confirm flag.

Implementation Details

Create extensions/penpot/src/tools/commit.ts (and, if it helps, src/commit.ts for the pure parts) and register all three tools.

`penpot_commit`

Parameters: file_id (string, required), dry_run (boolean, optional, default false) — dry_run returns the same preview without writing, so an agent can show the user what it is about to do.

Refuse to commit if the changeset for file_id is empty ("nothing staged").
Read the current file (get-file) to obtain the live revn/vern — do not trust a cached value. Update the changeset's base revn if it has moved since staging.
Build the body { id, sessionId, revn, vern, changes } and call update-file.
On revn-conflict: re-read the file, adopt the new revn, and retry once. If it conflicts a second time, return a clear error rather than looping.
On success: re-read the file and verify every staged change is present (colour/typography ids exist, shape ids exist). Report the observed new revn (from the re-read, never from the write response) in details as { fileId, previousRevn, newRevn, appliedChanges, verified: true }.
On failure: report the decoded error verbatim (including the server's explain for validation failures), state clearly that nothing was written, and keep the changeset staged so the caller can fix and retry.
On verified success: clear the changeset and return the preview list of what was applied.

`penpot_discard`

Parameters: file_id (string, required). Clears the staged changeset and returns the discarded preview (what was thrown away). Idempotent — discarding an empty changeset reports "nothing staged".

`penpot_status`

Parameters: file_id (string, optional — all staged files when omitted). Returns target file(s), base revn, change counts by type and the deterministic preview lines from describeChanges. Never writes.

Guard hook: commit must call the designated-target guard (next issue) before sending anything. Until that guard exists, implement the call site and a permissive placeholder so the next issue replaces exactly one function.

Acceptance Criteria

penpot_commit sends exactly one update-file call per successful commit, with {id, sessionId, revn, vern, changes}.
revn/vern are read immediately before the write; a stale cached value is never used.
A revn-conflict triggers exactly one refresh-and-retry, then a clear error if it happens again.
Success is confirmed by re-reading the file and checking each staged asset/shape id is present; details reports the post-write revn from the re-read.
The response's lagged array is never used to decide whether the write succeeded.
A rejected batch leaves the changeset staged and reports the decoded server error verbatim, stating that nothing was written.
dry_run: true returns the preview and makes no write call (asserted in tests).
penpot_discard clears the changeset, is idempotent, and reports what it discarded.
penpot_status never writes and reports per-file staged state.
Unit tests cover the conflict-retry path, the verification failure path, dry_run, empty-changeset refusal and discard, all with a stubbed client and no network access.

Test Plan

node --test extensions/penpot/src/tools/commit.test.ts

Live validation (requires PENPOT_URL/PENPOT_TOKEN, scratch project):

Stage three colours and two typographies against the scratch file.
Call penpot_status — confirm the preview lists exactly five changes.
Call penpot_commit with dry_run: true — confirm no write happens (file revn unchanged on re-read).
Call penpot_commit — confirm all five assets exist via penpot_get_file, and that the file's revision advanced exactly once (compare the revn before and after; a jump of more than one indicates more than one write).
Reload the file in the Penpot UI and confirm the assets are visible in the Assets panel.
Simulate staleness: stage a change, edit the file in the UI to move revn on, then commit — confirm the conflict path refreshes and the commit lands, and that the resulting revision advanced exactly once.

## Summary Implement `penpot_commit`, `penpot_discard` and `penpot_status`: apply the whole staged changeset as a single `update-file` call with correct `revn` handling, a preview of exactly what will change, and a post-write verification by re-reading the file. ## Background **Depends on:** #194 This is the only place the extension writes. Getting `revn` right here protects every later write path, because the semantics are counter-intuitive and were confirmed empirically on Penpot 2.17: - `update-file` body is `{ id, sessionId, revn, vern, changes }`. - The **response's `revn` is the pre-write revision**, not the new one. - A **lagged (lower) `revn` is accepted and applied** — it is *not* an optimistic lock. Writing twice with the same stale `revn` applied both writes. - Only a `revn` **greater** than the stored one errors (`revn-conflict`); a mismatched `vern` raises `vern-conflict`. - The response's `lagged` array echoes *other* revisions and is **not** an acknowledgement of your own write. Never treat it as confirmation. - A rejected batch increments nothing and applies nothing (atomic). Consequence: correctness depends on the extension reading `revn` immediately before writing and confirming success by **re-reading the file**, not by parsing the response. `sessionId` is a per-session UUID the server uses to attribute changes; generate one per extension session. ## Documentation Required A separate process downloads these into the listed folders before this issue is implemented. Check the folders for the actual reference material before starting. **`docs/reference/penpot-api/`** - `<PENPOT_URL>/api/main/doc/openapi.json` — the `update-file` command: body parameters, response fields (`revn`, `vern`, `lagged`) and the error types it can raise. - https://raw.githubusercontent.com/penpot/penpot/2.17.2/common/src/app/common/types/file.cljc — the file revision/version fields the commit reads and reports. - https://raw.githubusercontent.com/penpot/penpot/2.17.2/common/src/app/common/types/modifiers.cljc — the change-type/modifier union: what an `update-file` change list accepts, useful for validating a staged batch before sending. - https://help.penpot.app/technical-guide/integration/ — auth and command URL shape. - https://help.penpot.app/user-guide/ — expectations for what a designer sees after a change (used for the live validation wording). **`docs/reference/pi-coding-agent/`** and **`docs/reference/typebox/`** - https://pi.dev/docs/latest/extensions — tool registration and returning structured `details`. - https://github.com/sinclairzx81/typebox — boolean/string parameter schemas for a `dry_run`/`confirm` flag. ## Implementation Details Create `extensions/penpot/src/tools/commit.ts` (and, if it helps, `src/commit.ts` for the pure parts) and register all three tools. ### `penpot_commit` Parameters: `file_id` (string, required), `dry_run` (boolean, optional, default `false`) — `dry_run` returns the same preview without writing, so an agent can show the user what it is about to do. 1. Refuse to commit if the changeset for `file_id` is empty (`"nothing staged"`). 2. Read the current file (`get-file`) to obtain the live `revn`/`vern` — **do not trust a cached value**. Update the changeset's base `revn` if it has moved since staging. 3. Build the body `{ id, sessionId, revn, vern, changes }` and call `update-file`. 4. On `revn-conflict`: re-read the file, adopt the new `revn`, and retry **once**. If it conflicts a second time, return a clear error rather than looping. 5. On success: **re-read the file and verify** every staged change is present (colour/typography ids exist, shape ids exist). Report the observed new `revn` (from the re-read, never from the write response) in `details` as `{ fileId, previousRevn, newRevn, appliedChanges, verified: true }`. 6. On failure: report the decoded error verbatim (including the server's `explain` for validation failures), state clearly that nothing was written, and **keep the changeset staged** so the caller can fix and retry. 7. On verified success: clear the changeset and return the preview list of what was applied. ### `penpot_discard` Parameters: `file_id` (string, required). Clears the staged changeset and returns the discarded preview (what was thrown away). Idempotent — discarding an empty changeset reports `"nothing staged"`. ### `penpot_status` Parameters: `file_id` (string, optional — all staged files when omitted). Returns target file(s), base `revn`, change counts by type and the deterministic preview lines from `describeChanges`. Never writes. Guard hook: commit must call the designated-target guard (next issue) before sending anything. Until that guard exists, implement the call site and a permissive placeholder so the next issue replaces exactly one function. ## Acceptance Criteria - [ ] `penpot_commit` sends exactly one `update-file` call per successful commit, with `{id, sessionId, revn, vern, changes}`. - [ ] `revn`/`vern` are read immediately before the write; a stale cached value is never used. - [ ] A `revn-conflict` triggers exactly one refresh-and-retry, then a clear error if it happens again. - [ ] Success is confirmed by re-reading the file and checking each staged asset/shape id is present; `details` reports the post-write `revn` from the re-read. - [ ] The response's `lagged` array is never used to decide whether the write succeeded. - [ ] A rejected batch leaves the changeset staged and reports the decoded server error verbatim, stating that nothing was written. - [ ] `dry_run: true` returns the preview and makes no write call (asserted in tests). - [ ] `penpot_discard` clears the changeset, is idempotent, and reports what it discarded. - [ ] `penpot_status` never writes and reports per-file staged state. - [ ] Unit tests cover the conflict-retry path, the verification failure path, `dry_run`, empty-changeset refusal and discard, all with a stubbed client and no network access. ## Test Plan ```bash node --test extensions/penpot/src/tools/commit.test.ts ``` Live validation (requires `PENPOT_URL`/`PENPOT_TOKEN`, scratch project): 1. Stage three colours and two typographies against the scratch file. 2. Call `penpot_status` — confirm the preview lists exactly five changes. 3. Call `penpot_commit` with `dry_run: true` — confirm no write happens (file `revn` unchanged on re-read). 4. Call `penpot_commit` — confirm all five assets exist via `penpot_get_file`, and that the file's revision advanced **exactly once** (compare the `revn` before and after; a jump of more than one indicates more than one write). 5. Reload the file in the Penpot UI and confirm the assets are visible in the Assets panel. 6. Simulate staleness: stage a change, edit the file in the UI to move `revn` on, then commit — confirm the conflict path refreshes and the commit lands, and that the resulting revision advanced exactly once.

david added this to the Penpot: M2 - Staged changeset, token primitives, commit, safety milestone

2026-09-14 23:10:33 +00:00

david added the

feature

label

2026-09-14 23:10:33 +00:00

david referenced this issue

2026-09-14 23:10:48 +00:00

Implement the designated-target write guard with an allow/refuse matrix #196

david referenced this issue from a pull request that will close it,

2026-09-15 00:49:09 +00:00

issue-195: Implement penpot_commit with revn tracking, conflict retry and commit preview #228