Skip to content

fix(composer): make capability origin, status, and triggers legible - #3112

Open
benjaminshafii wants to merge 1 commit into
devfrom
fix/composer-capability-clarity
Open

fix(composer): make capability origin, status, and triggers legible#3112
benjaminshafii wants to merge 1 commit into
devfrom
fix/composer-capability-clarity

Conversation

@benjaminshafii

Copy link
Copy Markdown
Member

Problem

Three defects in how the composer's capability panel (the Plug icon — "Commands, skills, and MCPs") describes what a member has.

Fix 1 — status with no way out

MCP rows rendered Sign in needed / Offline / Issue as plain non-interactive <div>s. A member saw the problem and had no next step. Actionable rows are now buttons that route to where it can be fixed, reusing the panel's existing onOpenSettingsSection callback. Ready rows stay inert — nothing to do.

Fix 2 — the origin badge was backwards

Personal skills got a Local pill; skills that came from the organization got no badge at all — exactly backwards from what a member needs to know. Organization skills now carry a badge naming the marketplace they came from, so origin is legible for both cases without adding a row.

Fix 3 — trigger was meaningless for organization skills

Local skills derive trigger from frontmatter trigger:/when: or the first bullet under ## When to use. Organization skills derived it from the folder name, so the Trigger card in the detail modal showed noise for precisely the skills an enterprise member consumes.

Both paths now share one trigger rule (@openwork/types/skill-markdown), and the Trigger card hides when no trigger can be derived rather than showing something misleading.

Reviewer note on the shared helper

First attempt put a full YAML parser in packages/types to share the rule — which would have pulled a parser into the renderer bundle for one string field (apps/app has no yaml dependency today). Caught in review and reworked: the shared helper is now dependency-free, reads only trigger/when scalars, ignores indented keys and block scalars, and yields no trigger rather than a wrong one when it cannot read confidently. The server keeps using its own real YAML parseFrontmatter and passes the parsed data in. pnpm-lock.yaml diff is empty.

Verification

pnpm typecheck                        clean
pnpm --filter @openwork/app test      427 pass, 0 fail
pnpm --filter openwork-server test    520 pass, 10 skip, 0 fail

New tests cover the trigger reader (quoted values, when:, ignored nested keys, unreadable block scalar → undefined, ## When to use fallback) and the origin badge for both cases.

@vercel

vercel Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
openwork-app Ready Ready Preview, Comment Jul 25, 2026 1:36pm
openwork-den Ready Ready Preview, Comment Jul 25, 2026 1:36pm
openwork-den-worker-proxy Ready Ready Preview, Comment Jul 25, 2026 1:36pm
openwork-diagnostics Ready Ready Preview, Comment Jul 25, 2026 1:36pm
openwork-landing Ready Ready Preview, Comment, Open in v0 Jul 25, 2026 1:36pm

@benjaminshafii

Copy link
Copy Markdown
Member Author

Validation: real app, isolated Linux sandbox, matched A/B

All nine branches of this series were merged into one integration branch (f91884c3) and driven as a real user through CDP in a fresh Daytona sandbox. Baseline dev was run in a separate, identically-provisioned fresh sandbox with the same model (GPT-4o mini) so the comparison is apples-to-apples.

core-flow step clean dev (2fda6070) this series (f91884c3)
App boots to a usable session surface ✅ 105ms ✅ 94ms
User creates a fresh task in the workspace ✅ 674ms ✅ 662ms
User writes a message and runs it (real inference) ✅ 2768ms ✅ 111405ms
User closes and reopens; the session survives ❌ not persisted ❌ not persisted

Step 4 fails identically on clean dev (Session ses_… was not listed after reopen (not persisted)), so it is pre-existing on dev and not introduced here. Against the canonical core flow this series is inert, which is exactly the claim for a cleanup/wiring series.

Validated frames: core-flow-01-booted.png, core-flow-02-session-created.png, core-flow-03-task-response.png — including the assertion "Visible text does not include 'Something went wrong'".

A real regression this caught, in this series

Booting the real app initially failed with:

TypeError: Unknown file extension ".ts" for packages/types/src/skill-markdown.ts
→ "OpenWork server did not finish starting. Please restart OpenWork."

@openwork/types is a source-only package — every exports entry resolves to raw ./src/*.ts. apps/server is built with plain tsc (no bundling), so a runtime import there survives into emitted JS and Node cannot load it. Only the bun build-bundled files under src/opencode-plugins/ may import it.

pnpm typecheck and both bun suites passed straight through this, because bun loads .ts natively and Vite bundles for the renderer. Only booting the real product surfaced it. It is fixed in #3112, and that PR adds an automated guard that fails if any non-bundled apps/server file imports @openwork/types, plus a plain-node import proof of the built output.

Static verification on the merged result

pnpm typecheck                                        clean
pnpm --filter @openwork/app test                      435 pass,  0 fail
pnpm --filter openwork-server test                    526 pass,  0 fail
pnpm --filter @openwork-ee/den-web test                73 pass,  0 fail
pnpm --filter @openwork-ee/den-web build              succeeded
den-api desktop-policy suites (real MySQL)              9 pass,  0 fail

Honest gaps

  • No per-PR fraimz flow was authored; this is the canonical core-flow inertness proof plus the matched baseline, which is the right standard for a series that is mostly wiring, deletion, and policy gating.
  • The cloud prompt-card behavior in feat(desktop-policies): let organization prompts launch a named skill #3115 is covered by unit + real-DB API tests, not by a driven cloud fraimz — that needs the Den stack and a seeded org.
  • Local macOS pnpm dev could not be used: every worktree shares one userData dir, so app.requestSingleInstanceLock() makes any second instance quit before creating a window (CDP stays up, so it looks like a hang). Worth a separate DX fix.

@benjaminshafii

Copy link
Copy Markdown
Member Author

Correction to my validation note above

In the "Honest gaps" section I wrote that local macOS pnpm dev could not be used because all worktrees share one userData dir, so app.requestSingleInstanceLock() makes any second instance quit before creating a window.

That diagnosis was wrong. I tested it properly afterwards and disproved it:

  • With the lock successfully acquired (verified: the boot banner printed and no contention message appeared), the instance still produced 0 CDP page targets.
  • With an isolated profile and OPENWORK_ELECTRON_USE_MOCK_KEYCHAIN=1, still 0 page targets.
  • From the unmodified main checkout on dev with an isolated profile, still 0 page targets.

So dev Electron currently opens no window on this macOS host regardless of profile, keychain, or worktree. It is pre-existing, unrelated to this series, and I have not root-caused it. The same commits do open a window in a clean Linux sandbox, which is where the validation above was run — so the core-flow evidence in this comment stands unchanged.

The shared-profile collision is nonetheless real (two worktrees genuinely cannot both hold the lock, and the loser lingers with an open CDP port and no log line). That is fixed separately in #3124, which adds OPENWORK_DEV_PROFILE and makes lock contention explain itself and exit.

Flagging it because the original wording would have sent a reviewer chasing the wrong cause.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant