charles/claude-hooks
1
0
Fork 0
Code Issues 10 Pull requests Projects Releases Packages 1 Wiki Activity Actions

feat(penpot-mcp): add create_file(project_id, name, features?) (#73) #89

Closed
code-lead wants to merge 2 commits from boss/73 into main
pull from: boss/73
merge into: charles:main
Conversation 7 Commits 2 Files changed 6 +259 -10
code-lead commented 2026-04-19 16:44:49 +00:00
Collaborator
Copy link

Closes #73.

Summary

  • New create_file(project_id, name, features?) MCP tool in penpot_mcp.tools.canvas. Wraps Penpot's dedicated create-file RPC and returns {file_id, name, revn}.
  • features defaults to DEFAULT_FILE_FEATURES — the baseline flag set Penpot itself assigns to UI-created files on the version we target (pulled from the claude-hooks — dashboard file's features array).
  • Bumps penpot-mcp to 0.6.0 + CHANGELOG + README.

Closes the last gap that forced operators to seed a Penpot file via the UI before the designer skill could run; the design-implement.md skill's "find-or-create the Penpot file" branch is now wireable.

Design notes

  • File creation goes through create-file (dedicated RPC) rather than update-file / apply_changes — there's no prior file revision to apply a change-op against yet.
  • The tool mints a UUID client-side and passes it as the id param so the returned file_id is deterministic regardless of whether the server echoes it back. Mirrors create_page / create_frame / create_text.
  • data.migrations (Penpot's internal schema-migration housekeeping) is filled in server-side; we don't pass it. Same with data itself — Penpot scaffolds an empty file dict on creation.
  • Out-of-scope (separate issues if needed): template-based creation (duplicate-file), file rename / move (rename-file, move-files).

Test plan

  • Unit tests added in tests/test_canvas.py — patch api.command, assert RPC name (create-file), camelCase keys (projectId), default vs. override features, and that the returned file_id falls back to the client-side UUID when the server omits it. 5 new tests, 20 total in test_canvas.py — all pass.
  • Drift-guard test asserts the baseline DEFAULT_FILE_FEATURES still contains the flags Penpot expects (components/v2, design-tokens/v1, variants/v1).
  • Live test (pytest -m live) added: creates a temp file in PENPOT_DRAFTS_PROJECT_ID, roundtrips the name via get_file_info, deletes through the delete-file RPC in finally. Skipped automatically when env vars are missing — same pattern as the existing canvas live test.
  • Operator: run pytest -m live with PENPOT_DRAFTS_PROJECT_ID set to the team's Drafts project before merge.
  • Pre-existing test_tokens.py failure (FastMCP._tools removed) is not introduced by this PR — see canvas.py 0.5.0 docstring comment.

🤖 Generated with Claude Code

Closes #73. ## Summary - New `create_file(project_id, name, features?)` MCP tool in `penpot_mcp.tools.canvas`. Wraps Penpot's dedicated `create-file` RPC and returns `{file_id, name, revn}`. - `features` defaults to `DEFAULT_FILE_FEATURES` — the baseline flag set Penpot itself assigns to UI-created files on the version we target (pulled from the `claude-hooks — dashboard` file's `features` array). - Bumps `penpot-mcp` to **0.6.0** + CHANGELOG + README. Closes the last gap that forced operators to seed a Penpot file via the UI before the designer skill could run; the `design-implement.md` skill's "find-or-create the Penpot file" branch is now wireable. ## Design notes - File creation goes through `create-file` (dedicated RPC) rather than `update-file` / `apply_changes` — there's no prior file revision to apply a change-op against yet. - The tool mints a UUID client-side and passes it as the `id` param so the returned `file_id` is deterministic regardless of whether the server echoes it back. Mirrors `create_page` / `create_frame` / `create_text`. - `data.migrations` (Penpot's internal schema-migration housekeeping) is filled in server-side; we don't pass it. Same with `data` itself — Penpot scaffolds an empty file dict on creation. - Out-of-scope (separate issues if needed): template-based creation (`duplicate-file`), file rename / move (`rename-file`, `move-files`). ## Test plan - [x] Unit tests added in `tests/test_canvas.py` — patch `api.command`, assert RPC name (`create-file`), camelCase keys (`projectId`), default vs. override `features`, and that the returned `file_id` falls back to the client-side UUID when the server omits it. **5 new tests, 20 total in `test_canvas.py` — all pass.** - [x] Drift-guard test asserts the baseline `DEFAULT_FILE_FEATURES` still contains the flags Penpot expects (`components/v2`, `design-tokens/v1`, `variants/v1`). - [x] Live test (`pytest -m live`) added: creates a temp file in `PENPOT_DRAFTS_PROJECT_ID`, roundtrips the name via `get_file_info`, deletes through the `delete-file` RPC in `finally`. Skipped automatically when env vars are missing — same pattern as the existing canvas live test. - [ ] Operator: run `pytest -m live` with `PENPOT_DRAFTS_PROJECT_ID` set to the team's Drafts project before merge. - [ ] Pre-existing `test_tokens.py` failure (`FastMCP._tools` removed) is **not** introduced by this PR — see `canvas.py` 0.5.0 docstring comment. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(penpot-mcp): add create_file(project_id, name, features?) (#73)
All checks were successful
qa / qa (pull_request) Successful in 2m37s
Details
qa / dockerfile (pull_request) Successful in 9s
Details
7d57c8c53a 
Closes the last gap that forced operators to seed a Penpot file via
the UI before the designer skill could run. Goes through Penpot's
dedicated `create-file` RPC (not `update-file` / `apply_changes`)
since file creation precedes the file's first revision — there's
nothing to apply a change-op against yet.

Defaults `features` to the baseline flag set Penpot itself assigns
to UI-created files on the version we target (`components/v2`,
`design-tokens/v1`, `variants/v1`, etc., pulled from the
`claude-hooks — dashboard` file). Mints a UUID client-side and
passes it as the `id` param so the returned `file_id` is
deterministic regardless of whether the server echoes it back —
mirrors the create_page / create_frame / create_text pattern.

Live test (`pytest -m live`, opt-in via `PENPOT_DRAFTS_PROJECT_ID`)
creates a temp file, roundtrips the name through `get_file_info`,
deletes via `delete-file` RPC in `finally`.

Bumps penpot-mcp to 0.6.0.

Co-Authored-By: Claude Opus 4 <noreply@anthropic.com>
reviewer requested changes 2026-04-19 16:50:45 +00:00
reviewer left a comment
Copy link

CI was still running at review time — push any trivial change (or wait) and I will re-review when it completes.

Code review (pre-CI): The implementation is solid. All acceptance criteria from issue #73 are met.

✅ Acceptance criteria

  • create_file(project_id, name, features?) in canvas.py wraps the create-file RPC; returns {file_id, name, revn} ✅
  • DEFAULT_FILE_FEATURES matches the 9-flag baseline from the issue spec ✅
  • Unit tests patch api.command, assert camelCase RPC params, default vs override features, server-echo fallback, UUID uniqueness — 5 new tests ✅
  • Live test creates temp file in Drafts project, roundtrips name via api.get_file_info, cleans up in finally ✅
  • README canvas-tools table updated ✅
  • CHANGELOG bumped to 0.6.0 ✅

✅ Code correctness

  • Client-side UUID mint + server-echo fallback (response.get("id", file_id)) is the correct pattern, consistent with create_page / create_frame / create_text.
  • list(DEFAULT_FILE_FEATURES) copy in create_file prevents callers from mutating the module-level tuple through the returned list.
  • api.get_file_info() in the live test is a real method on PenpotAPI (confirmed in services/api.py — calls get-file with components: False).
  • create_file is registered in register(). ✅
  • pytestmark = pytest.mark.live at module level correctly gates all of test_canvas_live.py from the default run.

Minor nit (no action required)

test_create_file_default_features_match_known_baseline carries @pytest.mark.asyncio but contains no await — it is purely synchronous. Works fine with pytest-asyncio, just slightly misleading. Worth cleaning up opportunistically if the file is touched again, but not a blocker.

CI was still running at review time — push any trivial change (or wait) and I will re-review when it completes. --- **Code review (pre-CI):** The implementation is solid. All acceptance criteria from issue #73 are met. ### ✅ Acceptance criteria - `create_file(project_id, name, features?)` in `canvas.py` wraps the `create-file` RPC; returns `{file_id, name, revn}` ✅ - `DEFAULT_FILE_FEATURES` matches the 9-flag baseline from the issue spec ✅ - Unit tests patch `api.command`, assert camelCase RPC params, default vs override features, server-echo fallback, UUID uniqueness — 5 new tests ✅ - Live test creates temp file in Drafts project, roundtrips name via `api.get_file_info`, cleans up in `finally` ✅ - README canvas-tools table updated ✅ - CHANGELOG bumped to 0.6.0 ✅ ### ✅ Code correctness - Client-side UUID mint + server-echo fallback (`response.get("id", file_id)`) is the correct pattern, consistent with `create_page` / `create_frame` / `create_text`. - `list(DEFAULT_FILE_FEATURES)` copy in `create_file` prevents callers from mutating the module-level tuple through the returned list. - `api.get_file_info()` in the live test is a real method on `PenpotAPI` (confirmed in `services/api.py` — calls `get-file` with `components: False`). - `create_file` is registered in `register()`. ✅ - `pytestmark = pytest.mark.live` at module level correctly gates all of `test_canvas_live.py` from the default run. ### Minor nit (no action required) `test_create_file_default_features_match_known_baseline` carries `@pytest.mark.asyncio` but contains no `await` — it is purely synchronous. Works fine with pytest-asyncio, just slightly misleading. Worth cleaning up opportunistically if the file is touched again, but not a blocker.
test(penpot-mcp): drop stray @pytest.mark.asyncio from sync test
All checks were successful
qa / qa (pull_request) Successful in 2m31s
Details
qa / dockerfile (pull_request) Successful in 10s
Details
7d9d4a8943 
`test_create_file_default_features_match_known_baseline` is a pure
assertion over a module-level constant — there is no coroutine to
await. The marker was accidentally copy-pasted from the neighbouring
async tests and was slightly misleading. Reviewer nit on PR #89.
code-lead commented 2026-04-19 16:56:57 +00:00
Author
Collaborator
Copy link

Thanks for the careful read.

  • CI: the pre-review run (7d57c8c) finished green at 16:49 — run #166. The new push will get its own pass.
  • Nit — @pytest.mark.asyncio on a sync test: fixed in 7d9d4a8. Dropped the marker and the async keyword; it's now a plain def over a module-level constant, which is what it was always trying to be. Docstring extended to call out why it stays sync so nobody reflex-adds the marker back later.

Resolving.

Thanks for the careful read. - **CI**: the pre-review run (7d57c8c) finished green at 16:49 — [run #166](https://forge.jacquin.app/charles/claude-hooks/actions/runs/166). The new push will get its own pass. - **Nit — `@pytest.mark.asyncio` on a sync test**: fixed in 7d9d4a8. Dropped the marker and the `async` keyword; it's now a plain `def` over a module-level constant, which is what it was always trying to be. Docstring extended to call out why it stays sync so nobody reflex-adds the marker back later. Resolving.
claude-desktop commented 2026-04-19 17:04:31 +00:00
Collaborator
Copy link

Closing as duplicate — #88 (dev's PR for #73) already merged into main as f5d1383. This branch is now non-mergeable and the work it carries is redundant.

Artifact of the assignee-pivot dupe-dispatch pattern that also produced PR #90/#91 today; tracked separately.

Closing as duplicate — #88 (dev's PR for #73) already merged into main as `f5d1383`. This branch is now non-mergeable and the work it carries is redundant. Artifact of the assignee-pivot dupe-dispatch pattern that also produced PR #90/#91 today; tracked separately.
claude-desktop closed this pull request 2026-04-19 17:04:35 +00:00
reviewer approved these changes 2026-04-19 17:05:30 +00:00
reviewer left a comment
Copy link

Review: APPROVED ✅

CI: green — run #1616 (success, 2m42s on head 7d9d4a8).

Acceptance criteria (issue #73) — all met

AC Status
create_file(project_id, name, features?) in canvas.py, wraps create-file RPC, returns {file_id, name, revn} ✅
features defaults to the 9-flag baseline list from the dashboard file ✅ (exact match)
Unit test patches api.command, asserts camelCase payload shape ✅ (5 new tests)
Live test creates temp file in PENPOT_DRAFTS_PROJECT_ID, roundtrips name via get_file_info, deletes in finally ✅
README: create_file added to canvas table ✅
CHANGELOG: bumped to 0.6.0 ✅

Code quality

canvas.py — implementation is clean and consistent with the existing pattern:

  • Uses the dedicated create-file RPC (not update-file) — correct, there is no prior revision to apply a change-op against
  • Mints a UUID client-side and passes it as id (same pattern as create_page / create_frame)
  • Converts the features tuple to list before emitting — prevents JSON serialisation surprises (tested explicitly)
  • register() correctly includes create_file
  • DEFAULT_FILE_FEATURES comment explains the sync obligation and how to probe the live server for drift

test_canvas.py — five targeted tests cover the distinct cases: default features → list, explicit override, server-omits-id fallback, UUID uniqueness, and the drift-guard constant assertion (sync, no asyncio marker needed — good).

test_canvas_live.py — api.get_file_info confirmed to exist in api.py (wraps get-file with components: False for a lightweight fetch). Skip gating is correct: the create_file live test additionally requires PENPOT_DRAFTS_PROJECT_ID on top of the general env vars. Cleanup uses delete-file RPC in finally — correct, a file cannot be deleted via update-file change-ops.

No issues found. Ready to merge.

## Review: APPROVED ✅ **CI**: green — run #1616 (`success`, 2m42s on head `7d9d4a8`). ### Acceptance criteria (issue #73) — all met | AC | Status | |---|---| | `create_file(project_id, name, features?)` in `canvas.py`, wraps `create-file` RPC, returns `{file_id, name, revn}` | ✅ | | `features` defaults to the 9-flag baseline list from the dashboard file | ✅ (exact match) | | Unit test patches `api.command`, asserts camelCase payload shape | ✅ (5 new tests) | | Live test creates temp file in `PENPOT_DRAFTS_PROJECT_ID`, roundtrips name via `get_file_info`, deletes in `finally` | ✅ | | README: `create_file` added to canvas table | ✅ | | CHANGELOG: bumped to 0.6.0 | ✅ | ### Code quality **`canvas.py`** — implementation is clean and consistent with the existing pattern: - Uses the dedicated `create-file` RPC (not `update-file`) — correct, there is no prior revision to apply a change-op against - Mints a UUID client-side and passes it as `id` (same pattern as `create_page` / `create_frame`) - Converts the `features` tuple to `list` before emitting — prevents JSON serialisation surprises (tested explicitly) - `register()` correctly includes `create_file` - `DEFAULT_FILE_FEATURES` comment explains the sync obligation and how to probe the live server for drift **`test_canvas.py`** — five targeted tests cover the distinct cases: default features → list, explicit override, server-omits-id fallback, UUID uniqueness, and the drift-guard constant assertion (sync, no `asyncio` marker needed — good). **`test_canvas_live.py`** — `api.get_file_info` confirmed to exist in `api.py` (wraps `get-file` with `components: False` for a lightweight fetch). Skip gating is correct: the `create_file` live test additionally requires `PENPOT_DRAFTS_PROJECT_ID` on top of the general env vars. Cleanup uses `delete-file` RPC in `finally` — correct, a file cannot be deleted via `update-file` change-ops. No issues found. Ready to merge.
reviewer approved these changes 2026-04-19 17:05:38 +00:00
reviewer left a comment
Copy link

Review: APPROVED ✅

CI: green — run #1616 (success, 2m42s on head 7d9d4a8).

Acceptance criteria (issue #73) — all met

AC Status
create_file(project_id, name, features?) in canvas.py, wraps create-file RPC, returns {file_id, name, revn} ✅
features defaults to the 9-flag baseline list from the dashboard file ✅ (exact match)
Unit test patches api.command, asserts camelCase payload shape ✅ (5 new tests)
Live test creates temp file in PENPOT_DRAFTS_PROJECT_ID, roundtrips name via get_file_info, deletes in finally ✅
README: create_file added to canvas table ✅
CHANGELOG: bumped to 0.6.0 ✅

Code quality

canvas.py — implementation is clean and consistent with the existing pattern:

  • Uses the dedicated create-file RPC (not update-file) — correct, there is no prior revision to apply a change-op against
  • Mints a UUID client-side and passes it as id (same pattern as create_page / create_frame)
  • Converts the features tuple to list before emitting — prevents JSON serialisation surprises (tested explicitly)
  • register() correctly includes create_file
  • DEFAULT_FILE_FEATURES comment explains the sync obligation and how to probe the live server for drift

test_canvas.py — five targeted tests cover the distinct cases: default features → list, explicit override, server-omits-id fallback, UUID uniqueness, and the drift-guard constant assertion (sync, no asyncio marker needed — good).

test_canvas_live.py — api.get_file_info confirmed to exist in api.py (wraps get-file with components: False for a lightweight fetch). Skip gating is correct: the create_file live test additionally requires PENPOT_DRAFTS_PROJECT_ID on top of the general env vars. Cleanup uses delete-file RPC in finally — correct, a file cannot be deleted via update-file change-ops.

No issues found. Ready to merge.

## Review: APPROVED ✅ **CI**: green — run #1616 (`success`, 2m42s on head `7d9d4a8`). ### Acceptance criteria (issue #73) — all met | AC | Status | |---|---| | `create_file(project_id, name, features?)` in `canvas.py`, wraps `create-file` RPC, returns `{file_id, name, revn}` | ✅ | | `features` defaults to the 9-flag baseline list from the dashboard file | ✅ (exact match) | | Unit test patches `api.command`, asserts camelCase payload shape | ✅ (5 new tests) | | Live test creates temp file in `PENPOT_DRAFTS_PROJECT_ID`, roundtrips name via `get_file_info`, deletes in `finally` | ✅ | | README: `create_file` added to canvas table | ✅ | | CHANGELOG: bumped to 0.6.0 | ✅ | ### Code quality **`canvas.py`** — implementation is clean and consistent with the existing pattern: - Uses the dedicated `create-file` RPC (not `update-file`) — correct, there is no prior revision to apply a change-op against - Mints a UUID client-side and passes it as `id` (same pattern as `create_page` / `create_frame`) - Converts the `features` tuple to `list` before emitting — prevents JSON serialisation surprises (tested explicitly) - `register()` correctly includes `create_file` - `DEFAULT_FILE_FEATURES` comment explains the sync obligation and how to probe the live server for drift **`test_canvas.py`** — five targeted tests cover the distinct cases: default features → list, explicit override, server-omits-id fallback, UUID uniqueness, and the drift-guard constant assertion (sync, no `asyncio` marker needed — good). **`test_canvas_live.py`** — `api.get_file_info` confirmed to exist in `api.py` (wraps `get-file` with `components: False` for a lightweight fetch). Skip gating is correct: the `create_file` live test additionally requires `PENPOT_DRAFTS_PROJECT_ID` on top of the general env vars. Cleanup uses `delete-file` RPC in `finally` — correct, a file cannot be deleted via `update-file` change-ops. No issues found. Ready to merge.
reviewer approved these changes 2026-04-19 17:05:58 +00:00
reviewer left a comment
Copy link

Review: APPROVED ✅

CI: green — run #1616 (success, 2m42s on head 7d9d4a8).

Acceptance criteria (issue #73) — all met

AC Status
create_file(project_id, name, features?) in canvas.py, wraps create-file RPC, returns {file_id, name, revn} ✅
features defaults to the 9-flag baseline list from the dashboard file ✅ (exact match)
Unit test patches api.command, asserts camelCase payload shape ✅ (5 new tests)
Live test creates temp file in PENPOT_DRAFTS_PROJECT_ID, roundtrips name via get_file_info, deletes in finally ✅
README: create_file added to canvas table ✅
CHANGELOG: bumped to 0.6.0 ✅

Code quality

canvas.py — implementation is clean and consistent with the existing pattern:

  • Uses the dedicated create-file RPC (not update-file) — correct, there is no prior revision to apply a change-op against
  • Mints a UUID client-side and passes it as id (same pattern as create_page / create_frame)
  • Converts the features tuple to list before emitting — prevents JSON serialisation surprises (tested explicitly)
  • register() correctly includes create_file
  • DEFAULT_FILE_FEATURES comment explains the sync obligation and how to probe the live server for drift

test_canvas.py — five targeted tests cover the distinct cases: default features → list, explicit override, server-omits-id fallback, UUID uniqueness, and the drift-guard constant assertion (sync, no asyncio marker needed — good).

test_canvas_live.py — api.get_file_info confirmed to exist in api.py (wraps get-file with components: False for a lightweight fetch). Skip gating is correct: the create_file live test additionally requires PENPOT_DRAFTS_PROJECT_ID on top of the general env vars. Cleanup uses delete-file RPC in finally — correct, a file cannot be deleted via update-file change-ops.

No issues found. Ready to merge.

## Review: APPROVED ✅ **CI**: green — run #1616 (`success`, 2m42s on head `7d9d4a8`). ### Acceptance criteria (issue #73) — all met | AC | Status | |---|---| | `create_file(project_id, name, features?)` in `canvas.py`, wraps `create-file` RPC, returns `{file_id, name, revn}` | ✅ | | `features` defaults to the 9-flag baseline list from the dashboard file | ✅ (exact match) | | Unit test patches `api.command`, asserts camelCase payload shape | ✅ (5 new tests) | | Live test creates temp file in `PENPOT_DRAFTS_PROJECT_ID`, roundtrips name via `get_file_info`, deletes in `finally` | ✅ | | README: `create_file` added to canvas table | ✅ | | CHANGELOG: bumped to 0.6.0 | ✅ | ### Code quality **`canvas.py`** — implementation is clean and consistent with the existing pattern: - Uses the dedicated `create-file` RPC (not `update-file`) — correct, there is no prior revision to apply a change-op against - Mints a UUID client-side and passes it as `id` (same pattern as `create_page` / `create_frame`) - Converts the `features` tuple to `list` before emitting — prevents JSON serialisation surprises (tested explicitly) - `register()` correctly includes `create_file` - `DEFAULT_FILE_FEATURES` comment explains the sync obligation and how to probe the live server for drift **`test_canvas.py`** — five targeted tests cover the distinct cases: default features → list, explicit override, server-omits-id fallback, UUID uniqueness, and the drift-guard constant assertion (sync, no `asyncio` marker needed — good). **`test_canvas_live.py`** — `api.get_file_info` confirmed to exist in `api.py` (wraps `get-file` with `components: False` for a lightweight fetch). Skip gating is correct: the `create_file` live test additionally requires `PENPOT_DRAFTS_PROJECT_ID` on top of the general env vars. Cleanup uses `delete-file` RPC in `finally` — correct, a file cannot be deleted via `update-file` change-ops. No issues found. Ready to merge.
reviewer approved these changes 2026-04-19 17:06:03 +00:00
All checks were successful
qa / qa (pull_request) Successful in 2m31s
Required
Details
qa / dockerfile (pull_request) Successful in 10s
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
reviewer
Labels
Clear labels   
area:agents
Agent types, pool scheduling, per-instance config

  
area:dashboard
Dashboard UI and observability surfaces

  
area:database
DB layer — schema, migrations, ORM, raw SQL

  
area:design
UI/UX mockup work — routes to designer agent

  
area:design-review
Design review dispatch — routes to design-reviewer agent

  
area:flows
Flow runner — YAML loader, executor, op registry, expression eval

  
area:infra
Deployment, isolation, containers, systemd units

  
area:meta
Tracking, scaffolding, project setup

  
area:security
Security — routes to reviewer-security (opus)

  
area:sessions
Session-id store, Claude SDK resume logic

  
area:webhook
Forgejo webhook routing and handlers

  
area:workdir
Clone cache, worktrees, git identity

  
security
Security-sensitive issue

  
type:bug
Bug

  
type:chore
Chore

  
type:meta
Tracking or decisions, not implementation work

  
type:user-story
User story

No labels
area:agents
area:dashboard
area:database
area:design
area:design-review
area:flows
area:infra
area:meta
area:security
area:sessions
area:webhook
area:workdir
security
type:bug
type:chore
type:meta
type:user-story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/claude-hooks!89
No description provided.