PluginOS 0.5.0: TYPO3 Bootstrap feedback sweep#36
Open
LSDimi wants to merge 82 commits into
Open
Conversation
- add registerRule() to accumulate LintRule instances at runtime - add runLint() to dispatch code through all registered rules - re-export LintResult, LintRule, LintSeverity from types module - add registry tests (empty-registry and rule-aggregation cases)
- add noNotifyRule that flags figma.notify() calls as errors - detect calls with regex anchored to figma object only - report correct 1-based line numbers - add 4 unit tests covering flags, line numbers, and non-matches
- add rule that flags dots, hyphens, spaces in createVariable names - provide sanitized fix replacing invalid chars with underscores - add 4 tests covering dot, hyphen, space, and valid name cases
- add rule to detect hyphens in setPluginData and setSharedPluginData keys - report error with fix suggestion replacing hyphens with underscores - add 3 unit tests covering both method variants and valid keys
- add prefer-helpers rule that hints createStyledText when createText + loadFontAsync co-occur - add hint for bindSpacing when 3+ padding/itemSpacing setBoundVariable calls are found - add test coverage for both hints and the no-match case
- wire all 7 rules into index.ts at module load via registerRule() - add registry integration tests for no-notify and no-itemspacing-auto
- add PRELUDE_SOURCE string template with PluginOS namespace stub - add wrapScript() that prepends prelude to user scripts - export PRELUDE_VERSION resolved from package.json at runtime - add preludeLineCount for accurate source-map offset tracking - add failing-then-passing TDD tests (3 passing)
- add createStyledText async helper to prelude source - handles loadFontAsync, createText, setTextStyleIdAsync, setFillStyleIdAsync - throws descriptive error when required params missing - add helpers.test.ts with two vm-based tests (5 prelude tests total)
- add combineAsVariantsTiled to prelude source.ts IIFE - auto-computes cols from sqrt(cells.length) when not specified - sets layoutMode, layoutWrap, itemSpacing, counterAxisSpacing - fixes primaryAxisSizingMode FIXED and counterAxisSizingMode AUTO - resizes component set width based on cols, cellW, gutter - add test covering all layout fields and variant set reference
- add tileTopLevel to prelude source with cols, gutter, origin opts - add test covering 2-col grid layout with correct x/y and appendChild calls
- import wrapScript/PRELUDE_VERSION and runLint into server.ts - lint user code before execution, wrap with prelude before sending - return enriched payload: result, lint, preludeVersion, durationMs - add execute-with-lint integration tests (3 cases) - update server-tools test to read result from new nested shape
- add scripts/sync-recipes.ts with generateRecipesSection() and applyRecipes() exports - add __tests__/sync-recipes.test.ts with 2 tests covering all five helpers and header format - add sync-recipes npm script to package.json
- wrap bridge array result in { operations, total } envelope
- add two new tests covering non-empty and empty array cases
- update existing returns-operations test to assert new shape
INSTALL.md verification step and README.md architecture diagram both referenced specific operation counts (39 and 26) that drift. Reference list_operations as the source of truth instead.
…cipes Auto-formatting pass after the quality-helpers implementation. No behavior changes — pure whitespace and line-break normalization to satisfy the format:check CI gate. Also regenerated SKILL.md recipes section through prettier with the canonical spacing.
…8626-4rwp CI's 'npm audit --audit-level=high' fails because vitest <4.1.0 has a critical advisory (Vitest UI server can read/exec arbitrary files when running). We only invoke 'vitest run' (CLI), never the UI server, so the actual risk is zero — but CI's audit gate doesn't distinguish. Adding vitest@^4.1.8 + @vitest/coverage-v8@^4.1.8 to root overrides forces all workspace devDeps onto the patched line. All 261 tests pass locally on the bumped version with no breaking-change adjustments needed.
The prior override-only attempt didn't take effect because workspace devDeps with ^2.1.0 / ^1.0.0 pins were resolving directly to vitest 2.1.9 — npm install left them installed and the override is consulted only for transitive deps. Bumping each workspace's own devDependency forces resolution onto the 4.x line. Also restored the original lockfile baseline before applying so platform-specific entries (@esbuild/win32-*, sunos-x64, etc.) needed by the CI Linux runners are preserved. All 261 tests pass and npm audit reports 0 vulnerabilities.
CI's 'test:coverage' step crashes with ERR_MODULE_NOT_FOUND because vitest hoists to the root node_modules but its dynamic import of @vitest/coverage-v8 only finds the package under workspace-local node_modules — npm doesn't hoist it past the workspace boundary when only the workspace declares it. Adding both packages to the root devDependencies forces them to share the same hoisted location, so vitest's coverage chunk resolves cleanly. Local test:coverage now produces a clean Coverage summary with 100% across the board.
…8626-4rwp CI's 'npm audit --audit-level=high' fails because vitest <4.1.0 has a critical advisory (Vitest UI server can read/exec arbitrary files when running). We only invoke 'vitest run' (CLI), never the UI server, so the actual risk is zero — but CI's audit gate doesn't distinguish. Adding vitest@^4.1.8 + @vitest/coverage-v8@^4.1.8 to root overrides forces all workspace devDeps onto the patched line. All 261 tests pass locally on the bumped version with no breaking-change adjustments needed.
The prior override-only attempt didn't take effect because workspace devDeps with ^2.1.0 / ^1.0.0 pins were resolving directly to vitest 2.1.9 — npm install left them installed and the override is consulted only for transitive deps. Bumping each workspace's own devDependency forces resolution onto the 4.x line. Also restored the original lockfile baseline before applying so platform-specific entries (@esbuild/win32-*, sunos-x64, etc.) needed by the CI Linux runners are preserved. All 261 tests pass and npm audit reports 0 vulnerabilities.
CI's 'test:coverage' step crashes with ERR_MODULE_NOT_FOUND because vitest hoists to the root node_modules but its dynamic import of @vitest/coverage-v8 only finds the package under workspace-local node_modules — npm doesn't hoist it past the workspace boundary when only the workspace declares it. Adding both packages to the root devDependencies forces them to share the same hoisted location, so vitest's coverage chunk resolves cleanly. Local test:coverage now produces a clean Coverage summary with 100% across the board.
- add acquireLock/releaseLock helpers using O_EXCL atomic open - detect stale lockfiles by checking if owning PID is alive via kill(pid, 0) - preserve stale PID in returned LockAcquisition for caller awareness - add retry loop with configurable maxRetries and retryDelayMs - add full test suite: fresh acquire, live-PID block, release, stale takeover
- add RunInstallOptions with skipBridge for test isolation - validate --with-agent value against supported set (cursor, generic) - call runCursorAgent / runGenericAgent after bridge install - wire install subcommand in dispatcher to call runInstall - add TDD tests for unknown agent rejection and generic snippet
- update bin to dispatch any non-empty argv[2] to runCli (handles install, --help, --version, and unknown subcommands with error) - bare invocation (no argv) falls through to dist/index.js to start MCP server - add src/cli/index.ts as tsup entry so dist/cli/index.js is emitted on build
Top-of-page comparison table lets users find their install path in seconds (Claude Desktop / Claude Code / Cursor / Any other MCP host with time estimates). Each path is a self-contained 10-line section that references the new `npx pluginos install` CLI instead of a manual zip download. Troubleshooting section updated for the new Update needed copy-button workflow.
…date buttons Replaces the single-step mismatch view with a two-step actionable layout: step 1 copies the update command, step 2 copies the manifest path for re-import. Adds wireMismatchCopyButtons() and calls it at bootstrap.
Three coordinated cleanups so npm run check is green end to end: (1) add eslint-disable comments to the 4 CLI files since they legitimately emit user-facing stdout messages; (2) add explicit unknown[] types to mock-call array destructuring in install.test.ts so the test compiles under strict tsc; (3) prettier auto-format pass on INSTALL.md table separators and install.ts function signature wrapping.
…nter - add null guard for missing vars in bindSpacing - add frame null check and opts default in layoutSpaceBetween - handle VERTICAL layoutMode in layoutSpaceBetween TEXT sizing - fix no-sync-style-setters regex to use negative lookahead instead of [^=] - remove /g flag and lastIndex reset from no-notify PATTERN - guard prefer-helpers line calculation against findIndex returning -1 - pre-compile PADDING_REGEXES at module load to avoid per-line allocation - broaden readPackageVersion name check to include pluginos substring
…undation - guard acquireSingletonLock against self-PID to prevent self-reaping on PID recycling - capture INITIAL_PARENT_PID at module load to reliably detect orphan after Unix re-parenting - move clearTimeout to finally block in fetchStateJson to cover slow response bodies - wrap lockfile fh.write in try/finally to guarantee file handle close on write error - add self-PID guard at top of reapProcess as defensive check against accidental SIGKILL - use process.hrtime.bigint() in wait_for_reconnect deadline to avoid clock drift
webpack ts-loader doesn't auto-resolve .js extensions to .ts source the way vitest does, so the production build was failing with "Can't resolve './discovery.js'". Removing the explicit extension lets webpack walk the resolve.extensions list normally. Tests continue to pass; production webpack build now succeeds.
- Wrap all hardcoded dark-mode color tokens in var(--figma-color-*, fallback) chains - Add optional `command` field to AppState mismatch variant - Route mismatch-cmd population through renderUI instead of direct DOM mutation - Clamp formatElapsed input with Math.max(0, ms) to guard against clock skew - Preserve lastKnownPort across connecting→connecting transitions in computeNextStateFromStatus
The bridge-plugin's webpack ts-loader honored the tsconfig include field, which pulled in test files that import node:fs / node:url / node:path — modules that aren't in the browser lib + plugin-typings type set the production bundle needs. Excluding src/__tests__ from the production tsconfig keeps the build clean while vitest continues to type-check the tests via its own resolver.
- add visual feedback to mismatch copy buttons (✓ Copied / ⚠ Copy failed with 1500ms restore) - add structural type guard after JSON.parse in cursor agent to reject non-object top-level values - wrap installBridge filesystem ops in try/catch for structured error on partial failure
# Conflicts: # packages/bridge-plugin/src/ui-entry.ts
…ge.json Two smoke-test bugs surfaced when running the published-shape CLI: 1. defaultSourceDir hardcoded "<here>/../bridge" which resolves to packages/mcp-server/bridge — wrong. tsup chunks the install module into dist/install-XXX.js (one level up from dist/cli), so the correct resolution depends on where the runtime placed it. Walk a candidate list (dist/, dist/cli, src/cli) and return the first that exists. 2. readBridgeVersion parsed manifest.json for a `version` field — but Figma plugin manifests have no such field per the spec, so we always displayed v?. Switch to reading the mcp-server package.json (the existing single source of truth used by pluginos --version), which the bridge already ships in lockstep with. Tests updated to assert version is *some* semver string instead of the fixture's manifest value, since the manifest is no longer the source.
Context-handoff document for a follow-up session. Captures all 8 PRs state, every Gemini review response, the 2 smoke-test bugs caught locally (install source dir + version reading), and the manual test playbook still pending (two-process orphan reap, Figma end-to-end, Cursor merge tests). Includes project conventions and key commit references so a cold session can continue without re-deriving context.
The source bridge manifest declares `main: "dist/code.js"` and `ui: "dist/bootloader.html"` because the bridge-plugin workspace serves its manifest from the repo root with built files under `dist/`. When `pluginos install` copies all four files flat into `~/.pluginos/bridge/`, those paths break — Figma can't import the manifest because `dist/code.js` doesn't exist next to it. bundle-bridge.cjs now strips the `dist/` prefix from `main` and `ui` after copying so the bundled manifest matches the bundled layout. Adds a regression test that asserts the installed manifest's referenced files actually exist next to it.
Pulls every design + plan + handoff artifact from the four sweep
workstreams into a single branch ahead of the consolidated 0.5.0 PR.
- Specs and plans cherry-picked from docs/pr-{b,a1,a2,c}-*-spec branches
so reviewers see the design context next to the implementation
- New handoffs from this session:
- 2026-06-08-pr-a2-smoke-defects.md: D1/D2 theme + ops-count
regressions caught during smoke test (follow-up work)
- 2026-06-08-onboarding-test-plan.md: user-journey validation plan
for the announcement / README funnel
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces cross-platform singleton enforcement, state-file discovery, and a wait_for_reconnect MCP tool to eliminate orphan server processes and improve connection reliability. It also adds five PluginOS.* sandbox helpers, a pre-flight linter, a pluginos install CLI subcommand, and an actionable mismatch view. The review feedback highlights critical security and stability issues, particularly the need to guard against pid <= 1 in process.kill calls to prevent accidental signaling of process groups or system processes. Additionally, feedback points out that a corrupt lockfile could permanently block the server, and modifying parsed.mcpServers without verifying it is an object could trigger a TypeError.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| retryDelayMs?: number; | ||
| } | ||
|
|
||
| function isProcessAlive(pid: number): boolean { |
Contributor
There was a problem hiding this comment.
In Unix/Node.js, calling process.kill(pid, signal) with a pid of 0 sends the signal to all processes in the current process group, and negative PIDs send signals to process groups. To prevent accidental signaling of process groups or system processes (like init with PID 1), explicitly guard against pid <= 1.
function isProcessAlive(pid: number): boolean {
if (pid <= 1) return false;| } | ||
|
|
||
| export async function reapProcess(pid: number, opts: ReapOptions = {}): Promise<ReapResult> { | ||
| if (pid === process.pid) { |
Contributor
There was a problem hiding this comment.
To prevent accidental signaling of process groups (PID 0 or negative) or system processes (like init with PID 1), explicitly guard against pid <= 1 before attempting to signal or reap the process.
Suggested change
| if (pid === process.pid) { | |
| if (pid <= 1 || pid === process.pid) { |
| function defaultStateDir(): string { | ||
| return process.env.PLUGINOS_STATE_DIR ?? join(homedir(), ".pluginos"); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Explicitly guard against pid <= 1 in isProcessAlive to prevent accidental signaling of process groups or system processes.
Suggested change
| function isProcessAlive(pid: number): boolean { | |
| if (pid <= 1) return false; |
|
|
||
| let singletonInfo: SingletonInfo | null = null; | ||
| let currentParentAlive = true; | ||
| let parentLivenessInterval: NodeJS.Timeout | null = null; |
Contributor
There was a problem hiding this comment.
Explicitly guard against pid <= 1 in isProcessAlive to prevent accidental signaling of process groups or system processes.
function isProcessAlive(pid: number): boolean {
if (pid <= 1) return false;| if ((err as NodeJS.ErrnoException).code !== "EEXIST") throw err; | ||
|
|
||
| const oldPid = await readPidFromLockfile(path); | ||
| if (oldPid !== null && !isProcessAlive(oldPid)) { |
Contributor
There was a problem hiding this comment.
If the lockfile is empty or contains invalid/corrupt data, readPidFromLockfile returns null. Currently, the code only unlinks the lockfile if oldPid !== null. This means a corrupt or empty lockfile will permanently block the server from starting. Consider unlinking the lockfile if oldPid is null as well.
Suggested change
| if (oldPid !== null && !isProcessAlive(oldPid)) { | |
| if (oldPid === null || !isProcessAlive(oldPid)) { |
Comment on lines
+69
to
+71
| mcpServers.pluginos = PLUGINOS_ENTRY; | ||
| parsed.mcpServers = mcpServers; | ||
|
|
Contributor
There was a problem hiding this comment.
If parsed.mcpServers exists but is not an object (e.g., a string or primitive), attempting to set mcpServers.pluginos will throw a TypeError in strict mode. Safely check that parsed.mcpServers is a non-null object before casting and modifying it.
Suggested change
| mcpServers.pluginos = PLUGINOS_ENTRY; | |
| parsed.mcpServers = mcpServers; | |
| const mcpServers = (typeof parsed.mcpServers === "object" && parsed.mcpServers !== null && !Array.isArray(parsed.mcpServers)) | |
| ? (parsed.mcpServers as Record<string, unknown>) | |
| : {}; | |
| mcpServers.pluginos = PLUGINOS_ENTRY; | |
| parsed.mcpServers = mcpServers; |
This was referenced Jun 9, 2026
PR-B's recipes section pushed the skill from comfortably under budget to 1163 tokens — 13 over the CI limit. Tightened three high-fat sections (`mcp__Figma__*` warning, "never mix" rule, operations quick-list) without losing any guidance. Now 1126 tokens, 24 of headroom restored.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates four workstreams addressing the 2026-06-03 TYPO3 Bootstrap user-research feedback into a single 0.5.0 release. Replaces PRs #26–#31, #33, #34, which are now closed with pointers here.
~/.pluginos/server.pid.lock, discovery file~/.pluginos/state.json,wait_for_reconnectMCP tool, aggressive SIGTERM→SIGKILL takeover.PluginOS.*sandbox helpers (createStyledText,bindSpacing,combineAsVariantsTiled,tileTopLevel,layoutSpaceBetween), 7-rule pre-flight linter, skill recipes section.AppStatediscriminated union, idempotentrenderUI(state)orchestrator, Figma CSS-var theme fallback chain, activity log polish (visible entries 5→10).pluginos installCLI subcommand with--with-agent cursor|generic, bundled bridge indist/bridge/, INSTALL.md restructure per-agent, actionable mismatch view with copy buttons.Diff stats: 77 files, +4555/−1426. 229 tests across 4 workspaces — all green.
What's also in here
44af5b4,514b7b9/13daba3,fca2ca6/68890da,4a9223b). Includes a HIGH-severity Linux orphan-heartbeat fix (captureINITIAL_PARENT_PIDat module load, not on demand —process.ppidreturns 1 after re-parenting on Unix).00049ab,1919ab5):pluginos installcouldn't find the bundled bridge (tsup chunked the install module one level deeper than expected). Now walks a candidate-list.v?(Figma plugin manifests have noversionfield). Now reads from mcp-server'spackage.json.main: \"dist/code.js\"but install copies files flat.bundle-bridge.cjsnow strips thedist/prefix during bundle; regression test added.Known follow-ups (NOT in this PR — track separately)
See `docs/superpowers/handoffs/2026-06-08-pr-a2-smoke-defects.md` for details:
These can be tackled as a 0.5.1 patch; they don't gate review of this PR's scope.
How to review
Order recommended in the original sweep handoff (`docs/superpowers/handoffs/2026-06-05-pr-sweep-handoff.md`):
To explore by workstream, the merge commits (`3558b29`, `29a0bcf`, `0a3963e` for B/A2/C, with A1 merged into integration earlier) preserve provenance.
Smoke tests run
A full onboarding test plan for the end-to-end user journey (cold-visitor → first frame in Figma) is in `docs/superpowers/handoffs/2026-06-08-onboarding-test-plan.md`.
Test plan