issue-189: Implement the penpot_whoami tool and wire the extension entry point #222

Merged

david merged 3 commits from feature/issue-189/implement-the-penpot-whoami-tool-and-wire-the-exte into main

2026-09-14 23:47:28 +00:00

david commented

2026-09-14 23:47:28 +00:00

Owner

189: Implement the penpot_whoami tool and wire the extension entry point

Closes #189 — 189.

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 9m 33s.

Review findings

I'll start by exploring the repository and understanding the current state.Now let me apply the High severity fix to whoami.ts:Now the Medium and Low findings:All gates re-run: penpot suite 68/68 pass, strict tsc --noEmit over extensions/penpot/src/**/*.ts (sources + tests) clean, root npm test unchanged at 194/195 with the same pre-existing local-install.test.mjs:1875 failure.

Review Findings — 189

Summary

Fixed every finding in the anchored penpot change. The High finding is resolved by making penpot_whoami signal failure the way pi actually observes it: errorResult was replaced with a failTool helper that throws (throw new Error(error.message)), since pi only sets isError for execute() calls that throw — AgentToolResult has no isError field and returning a value never sets the flag. The now-inert isError interface member, its docstrings, and the six result.isError / details.error assertions in whoami.test.ts were removed, and the failure tests now assert on the thrown error's message via a runToolFailure() helper. The unreachable details.error payload was dropped from WhoamiToolDetails accordingly. Medium 1 was fixed by re-scoping rule 2 of the decodeErrorBody docstring (raw Transit excerpt omitted when status guidance applies; body still on PenpotError.rawBody). Medium 2 was fixed by updating both stale README descriptions (README.md:38, README.md:143) to name penpot_whoami. Low was fixed by adding an [Unreleased] → Added CHANGELOG bullet covering the tool, PENPOT_URL/PENPOT_TOKEN, and the Authorization: Token <token> scheme.

One part of the High finding's suggested fix was deliberately not applied: extensions/forgejo/src/index.ts:143 uses the same inert isError pattern, but it is a separate extension with its own jest suite and 69 isError assertions across extensions/forgejo/tests/tools.test.ts; rewriting it is out of scope for issue 189 and too large/risky for this single pass, so it is left for its own change. Nothing from the review remains unresolved within the penpot change itself.

Critical

High

extensions/penpot/src/tools/whoami.ts:119 — the failure path returns isError: true on the result object, but pi only marks a tool call as failed when execute throws: AgentToolResult has no isError field (pi-agent-core/dist/types.d.ts:316), executePreparedToolCall returns { result, isError: false } for any normally-returned value, and the official docs state "Returning a value never sets the error flag regardless of what properties you include in the return object" (@earendil-works/pi-coding-agent/docs/extensions.md:1984). Consequently the docstring at whoami.ts:65-68 and the assertions at whoami.test.ts:157,174,197,214,237 encode behaviour that does not exist, and the tool's headline failure (missing/expired PENPOT_TOKEN) is reported to the model/UI as a successful call. Suggested fix: in errorResult (whoami.ts:107), throw new Error(error.message) so pi catches it, sets isError: true and reports the message without a stack trace, then update the tests to assert.rejects; or, if the details payload must be retained, keep returning the text but delete the isError interface/claims and stop asserting the flag. extensions/forgejo/src/index.ts:143 shares this inert pattern, so fix both together to avoid re-introducing it in the later penpot tools.

Medium

extensions/penpot/src/errors.ts:385 — the decodeErrorBody docstring now contradicts the implementation: rule 2 says Transit bodies "always include the raw excerpt", but the new status-guidance branch (errors.ts:402-412) deliberately omits it for 401/403/404 bodies. Suggested fix: reword rule 2 so the raw-excerpt statement is scoped to Transit bodies without status guidance, and note that the untouched body is still carried on PenpotError.rawBody.
README.md:38 — the root README still describes penpot as "scaffold only: no tools registered yet", which this change makes false now that penpot_whoami is registered; the repository-layout entry at README.md:143 repeats the same stale wording. Suggested fix: update both to name the registered tool, e.g. "penpot_whoami (verify PENPOT_URL/PENPOT_TOKEN) — more tools to follow; needs PENPOT_URL/PENPOT_TOKEN".

Low

CHANGELOG.md:7 — the [Unreleased] section has no entry for the first user-visible penpot tool, while comparable extension work is recorded (e.g. the mongodb registration at CHANGELOG.md:11). Suggested fix: add an ### Added bullet describing penpot_whoami, the PENPOT_URL/PENPOT_TOKEN configuration, and the Authorization: Token <token> scheme.

Documentation

Updated files:

README.md

## 189: Implement the penpot_whoami tool and wire the extension entry point Closes #189 — [189](https://git.excelera.net/david/pi-extensions-and-skills/issues/189). > 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 9m 33s. ## Review findings I'll start by exploring the repository and understanding the current state.Now let me apply the High severity fix to `whoami.ts`:Now the Medium and Low findings:All gates re-run: penpot suite **68/68 pass**, strict `tsc --noEmit` over `extensions/penpot/src/**/*.ts` (sources + tests) clean, root `npm test` unchanged at 194/195 with the same pre-existing `local-install.test.mjs:1875` failure. # Review Findings — 189 ## Summary Fixed every finding in the anchored penpot change. The High finding is resolved by making `penpot_whoami` signal failure the way pi actually observes it: `errorResult` was replaced with a `failTool` helper that throws (`throw new Error(error.message)`), since pi only sets `isError` for `execute()` calls that throw — `AgentToolResult` has no `isError` field and returning a value never sets the flag. The now-inert `isError` interface member, its docstrings, and the six `result.isError` / `details.error` assertions in `whoami.test.ts` were removed, and the failure tests now assert on the thrown error's message via a `runToolFailure()` helper. The unreachable `details.error` payload was dropped from `WhoamiToolDetails` accordingly. Medium 1 was fixed by re-scoping rule 2 of the `decodeErrorBody` docstring (raw Transit excerpt omitted when status guidance applies; body still on `PenpotError.rawBody`). Medium 2 was fixed by updating both stale README descriptions (`README.md:38`, `README.md:143`) to name `penpot_whoami`. Low was fixed by adding an `[Unreleased] → Added` CHANGELOG bullet covering the tool, `PENPOT_URL`/`PENPOT_TOKEN`, and the `Authorization: Token <token>` scheme. One part of the High finding's suggested fix was deliberately **not** applied: `extensions/forgejo/src/index.ts:143` uses the same inert `isError` pattern, but it is a separate extension with its own jest suite and 69 `isError` assertions across `extensions/forgejo/tests/tools.test.ts`; rewriting it is out of scope for issue 189 and too large/risky for this single pass, so it is left for its own change. Nothing from the review remains unresolved within the penpot change itself. ## Critical ## High - [x] extensions/penpot/src/tools/whoami.ts:119 — the failure path returns `isError: true` on the result object, but pi only marks a tool call as failed when `execute` **throws**: `AgentToolResult` has no `isError` field (`pi-agent-core/dist/types.d.ts:316`), `executePreparedToolCall` returns `{ result, isError: false }` for any normally-returned value, and the official docs state "Returning a value never sets the error flag regardless of what properties you include in the return object" (`@earendil-works/pi-coding-agent/docs/extensions.md:1984`). Consequently the docstring at whoami.ts:65-68 and the assertions at whoami.test.ts:157,174,197,214,237 encode behaviour that does not exist, and the tool's headline failure (missing/expired `PENPOT_TOKEN`) is reported to the model/UI as a successful call. Suggested fix: in `errorResult` (whoami.ts:107), `throw new Error(error.message)` so pi catches it, sets `isError: true` and reports the message without a stack trace, then update the tests to `assert.rejects`; or, if the details payload must be retained, keep returning the text but delete the `isError` interface/claims and stop asserting the flag. `extensions/forgejo/src/index.ts:143` shares this inert pattern, so fix both together to avoid re-introducing it in the later penpot tools. ## Medium - [x] extensions/penpot/src/errors.ts:385 — the `decodeErrorBody` docstring now contradicts the implementation: rule 2 says Transit bodies "always include the raw excerpt", but the new status-guidance branch (errors.ts:402-412) deliberately omits it for 401/403/404 bodies. Suggested fix: reword rule 2 so the raw-excerpt statement is scoped to Transit bodies *without* status guidance, and note that the untouched body is still carried on `PenpotError.rawBody`. - [x] README.md:38 — the root README still describes penpot as "scaffold only: no tools registered yet", which this change makes false now that `penpot_whoami` is registered; the repository-layout entry at README.md:143 repeats the same stale wording. Suggested fix: update both to name the registered tool, e.g. "`penpot_whoami` (verify `PENPOT_URL`/`PENPOT_TOKEN`) — more tools to follow; needs `PENPOT_URL`/`PENPOT_TOKEN`". ## Low - [x] CHANGELOG.md:7 — the `[Unreleased]` section has no entry for the first user-visible penpot tool, while comparable extension work is recorded (e.g. the mongodb registration at CHANGELOG.md:11). Suggested fix: add an `### Added` bullet describing `penpot_whoami`, the `PENPOT_URL`/`PENPOT_TOKEN` configuration, and the `Authorization: Token <token>` scheme. ## Documentation Updated files: - README.md

david added 3 commits

2026-09-14 23:47:28 +00:00