issue-197: Validate asset creation, atomicity and write safety against the live instance #230

Merged

david merged 3 commits from feature/issue-197/validate-asset-creation-atomicity-and-write-safety into main

2026-09-15 02:55:24 +00:00

david commented

2026-09-15 02:55:23 +00:00

Owner

197: Validate asset creation, atomicity and write safety against the live instance

Closes #197 — 197.

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 55s.

Review findings

Now let me apply the fixes to the harness. Starting with the High-severity scratch-target guard and the Low-severity exit handling.Now add the Medium-severity check for a nominated unrelated project:Now let me create the committed stub server (Medium finding):All checks pass. Now let me verify the new scratch-target guard actually refuses a non-scratch file name:Now update findings.md to reflect the new guard, the committed stub, and accurate evidence wording:Now let me run the project's test gate to confirm no regressions:All remediations are applied and verified. Final annotated findings below.

Review Findings — 197

Summary

Remediated the four code-level findings. The harness now reads the target file before any write and refuses unless its name carries pi-penpot-scratch (or PENPOT_SCRATCH_CONFIRM=1 acknowledges a differently named disposable file), and the false "can never be aimed at a file with real designs" claim was corrected in both the script header and findings.md. A committed stub (extensions/penpot/scripts/live-write-stub.mjs) now boots a local 2.17-shaped server and runs the harness against it, so the control flow is reproducible; the harness gained check 2b for the nominated-project read-then-refuse path (one get-file, zero update-file); and it sets process.exitCode instead of calling process.exit(). A green stub run now reports 35 passing assertions. The Critical live-evidence finding remains unresolved: no PENPOT_TOKEN exists in this environment, so none of the five live acceptance checks could be run against https://penpot.excelera.net — the issue must stay open/blocked until a token holder runs the harness and pastes the per-check revn/call evidence.

Critical

extensions/penpot/findings.md:282 — Acceptance criteria 1–5 are unmet: every row of the "Check status (live)" table is not run — no token, and the environment section (:209) confirms no write check ever touched https://penpot.excelera.net. The issue's deliverable is live evidence (single-revision commit with revn +1, UI Assets-panel confirmation, no-write refusal, atomic rejected batch with verbatim explain, stale-revn success), and none of it exists; findings.md only contains a stub run and an OpenAPI static cross-check. Suggested fix: run the committed harness with a valid PENPOT_TOKEN + scratch file id against the instance (PENPOT_URL=https://penpot.excelera.net PENPOT_TOKEN=… PENPOT_SCRATCH_FILE_ID=… node extensions/penpot/scripts/live-write-check.mjs), paste the captured per-check revn before/after and call sequence into findings.md, and have a human perform the UI Assets-panel reload for check 1. No Penpot credentials are present in this pipeline's environment either, so if the token cannot be supplied this issue must be left open/blocked rather than merged as complete — it cannot be satisfied by a code change alone.

High

extensions/penpot/scripts/live-write-check.mjs:9 — The header claims it "can never be aimed at a file with real designs", and findings.md:247 repeats "it cannot be aimed at a file with real designs", but the script only enforces that PENPOT_SCRATCH_FILE_ID is set; it then calls penpot_authorize_target on whatever id it was given and writes to it. A wrong id (or a copy-paste of a real file id) will happily create pi-197-* assets in a real design — the exact failure the write guard exists to prevent. Suggested fix: before staging, read the file (the script already reads it) and abort with exit 2 unless its name/project matches the disposable scratch target (e.g. name contains pi-penpot-scratch), or require an explicit PENPOT_SCRATCH_CONFIRM=1 acknowledgement; then correct the claim in the file header and findings.md:247.

Medium

extensions/penpot/findings.md:265 — The "All 28 assertions passed" stub exercise is not reproducible: the stub server is not committed and the harness is not in any npm test glob (package.json tests only scripts/local-install.test.mjs and extensions/penpot/src/**/*.test.ts). Anyone reading the evidence cannot re-run the only validation this change actually performed. Suggested fix: commit the stub (e.g. extensions/penpot/scripts/live-write-stub.mjs) or add an in-process *.test.ts that boots a stub and drives the four checks, and wire it into the test script so the harness's control flow is covered by CI.
extensions/penpot/scripts/live-write-check.mjs:223 — Check 2 only exercises the "nothing designated at all" refusal, where isWriteAllowed returns before any I/O. The drift note (findings.md:296) documents that a refusal with a nominated project does send one get-file read; that path — the one the README/comment were rewritten for — is asserted nowhere, so the harness's "no request sent" line proves less than the AC's "refuses ... before any request is sent" reading might suggest. Suggested fix: add a check that nominates an unrelated project, commits against the scratch file, and asserts exactly one get-file and zero update-file calls with refused === true.

Low

extensions/penpot/scripts/live-write-check.mjs:470 — process.exit() is called immediately after console.log, which can truncate pending stdout writes when the harness is piped (e.g. CI capturing output), potentially dropping the very evidence the script exists to produce. Suggested fix: set process.exitCode = failures === 0 ? 0 : 1 and let the process exit naturally, or await new Promise((r) => process.stdout.write("", r)) before exiting.

Documentation

Updated files:

docs/PENPOT_SKILL_DESIGN.md

## 197: Validate asset creation, atomicity and write safety against the live instance Closes #197 — [197](https://git.excelera.net/david/pi-extensions-and-skills/issues/197). > 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 55s. ## Review findings Now let me apply the fixes to the harness. Starting with the High-severity scratch-target guard and the Low-severity exit handling.Now add the Medium-severity check for a nominated unrelated project:Now let me create the committed stub server (Medium finding):All checks pass. Now let me verify the new scratch-target guard actually refuses a non-scratch file name:Now update `findings.md` to reflect the new guard, the committed stub, and accurate evidence wording:Now let me run the project's test gate to confirm no regressions:All remediations are applied and verified. Final annotated findings below. # Review Findings — 197 ## Summary Remediated the four code-level findings. The harness now reads the target file before any write and refuses unless its name carries `pi-penpot-scratch` (or `PENPOT_SCRATCH_CONFIRM=1` acknowledges a differently named disposable file), and the false "can never be aimed at a file with real designs" claim was corrected in both the script header and `findings.md`. A committed stub (`extensions/penpot/scripts/live-write-stub.mjs`) now boots a local 2.17-shaped server and runs the harness against it, so the control flow is reproducible; the harness gained check 2b for the nominated-project read-then-refuse path (one `get-file`, zero `update-file`); and it sets `process.exitCode` instead of calling `process.exit()`. A green stub run now reports 35 passing assertions. The **Critical** live-evidence finding remains unresolved: no `PENPOT_TOKEN` exists in this environment, so none of the five live acceptance checks could be run against `https://penpot.excelera.net` — the issue must stay open/blocked until a token holder runs the harness and pastes the per-check `revn`/call evidence. ## Critical - [ ] `extensions/penpot/findings.md:282` — Acceptance criteria 1–5 are unmet: every row of the "Check status (live)" table is `not run — no token`, and the environment section (`:209`) confirms no write check ever touched `https://penpot.excelera.net`. The issue's deliverable is live evidence (single-revision commit with `revn` +1, UI Assets-panel confirmation, no-write refusal, atomic rejected batch with verbatim `explain`, stale-`revn` success), and none of it exists; `findings.md` only contains a stub run and an OpenAPI static cross-check. Suggested fix: run the committed harness with a valid `PENPOT_TOKEN` + scratch file id against the instance (`PENPOT_URL=https://penpot.excelera.net PENPOT_TOKEN=… PENPOT_SCRATCH_FILE_ID=… node extensions/penpot/scripts/live-write-check.mjs`), paste the captured per-check `revn` before/after and call sequence into `findings.md`, and have a human perform the UI Assets-panel reload for check 1. No Penpot credentials are present in this pipeline's environment either, so if the token cannot be supplied this issue must be left open/blocked rather than merged as complete — it cannot be satisfied by a code change alone. ## High - [x] `extensions/penpot/scripts/live-write-check.mjs:9` — The header claims it "can never be aimed at a file with real designs", and `findings.md:247` repeats "it cannot be aimed at a file with real designs", but the script only enforces that `PENPOT_SCRATCH_FILE_ID` is *set*; it then calls `penpot_authorize_target` on whatever id it was given and writes to it. A wrong id (or a copy-paste of a real file id) will happily create `pi-197-*` assets in a real design — the exact failure the write guard exists to prevent. Suggested fix: before staging, read the file (the script already reads it) and abort with exit 2 unless its `name`/project matches the disposable scratch target (e.g. name contains `pi-penpot-scratch`), or require an explicit `PENPOT_SCRATCH_CONFIRM=1` acknowledgement; then correct the claim in the file header and `findings.md:247`. ## Medium - [x] `extensions/penpot/findings.md:265` — The "All 28 assertions passed" stub exercise is not reproducible: the stub server is not committed and the harness is not in any `npm test` glob (`package.json` tests only `scripts/local-install.test.mjs` and `extensions/penpot/src/**/*.test.ts`). Anyone reading the evidence cannot re-run the only validation this change actually performed. Suggested fix: commit the stub (e.g. `extensions/penpot/scripts/live-write-stub.mjs`) or add an in-process `*.test.ts` that boots a stub and drives the four checks, and wire it into the `test` script so the harness's control flow is covered by CI. - [x] `extensions/penpot/scripts/live-write-check.mjs:223` — Check 2 only exercises the "nothing designated at all" refusal, where `isWriteAllowed` returns before any I/O. The drift note (`findings.md:296`) documents that a refusal with a nominated project *does* send one `get-file` read; that path — the one the README/comment were rewritten for — is asserted nowhere, so the harness's "no request sent" line proves less than the AC's "refuses ... before any request is sent" reading might suggest. Suggested fix: add a check that nominates an unrelated project, commits against the scratch file, and asserts exactly one `get-file` and zero `update-file` calls with `refused === true`. ## Low - [x] `extensions/penpot/scripts/live-write-check.mjs:470` — `process.exit()` is called immediately after `console.log`, which can truncate pending stdout writes when the harness is piped (e.g. CI capturing output), potentially dropping the very evidence the script exists to produce. Suggested fix: set `process.exitCode = failures === 0 ? 0 : 1` and let the process exit naturally, or `await new Promise((r) => process.stdout.write("", r))` before exiting. ## Documentation Updated files: - docs/PENPOT_SKILL_DESIGN.md

david added 3 commits

2026-09-15 02:55:23 +00:00