Skip to content

feat: show skill startup diagnostics - #200

Merged
kahme247 merged 6 commits into
kahme247:mainfrom
andrebrait:feat/skill-startup-diagnostics
Oct 5, 2026
Merged

kahme247 merged 6 commits into
kahme247:mainfrom
andrebrait:feat/skill-startup-diagnostics

Conversation

@andrebrait

@andrebrait andrebrait commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

What

Show OMP's skill conflict and redundant-installation diagnostics above the composer when its RPC session starts. Details show the resolved default, namespaced variants, retained identical copies, paths, sources and selection reason.

Add an × button that dismisses the notice for the session until the reported diagnostics change, a Turn this off action, and a native Skill startup notices setting. Manual inspection stays available for a selected running session in Settings → Extensions & Tools → Skills while automatic notices are off; it never resumes a stopped session. English, Japanese and Simplified Chinese labels are included.

Why

Companion to can1357/oh-my-pi#14148. OMP remains the only resolver: this consumes the new RPC query/state/update frames, without a second collision detector or a separate browser preference. An empty new-chat page does not spawn OMP solely for diagnostics. Older binaries report manual diagnostics as unavailable rather than falsely clean.

Testing

  • npm test: 1,128 passed, 5 skipped, zero failures.
  • npm run typecheck: passed. npm run lint: zero errors, 11 unchanged baseline warnings (not a zero-warning gate).
  • Launched isolated Next dev with the modified real OMP source runtime and conflicting/identical fixture skills, with no copied credentials or model inference.
  • Observed the notice before the first model prompt; inspected selected/variant/mirror details; disabled notices through the UI and confirmed native YAML persisted false; a separate OMP process observed false. Manual inspection remained available while off.
  • Verified cross-client setting updates, the native YAML checkbox reaching a running OMP through its config watcher, and a conflict-to-clean reload updating the browser. Full hook tests cover missed-frame hydration, held state versus newer SSE, a setter's own event beating its HTTP response, stale session responses, and single-stream fresh-session attachment.
  • Visually inspected midnight and light dialogs and the 390px mobile layout; no horizontal overflow. Used the existing accessible Dialog controls and keyboard Escape.
  • Rendered the notice with 6 conflicts and 52 redundant copies at 390px: one row, text column 124px wide (wider than with the old Turn off startup notices label), no overflow; × removes it.
  • With no live wrapper, the real getter and setter endpoints return 409 and leave the session stopped. With the real installed older OMP binary, manual inspection shows the localized unavailable message rather than a clean result.
  • No production next build or deployment was performed, per the development rules.

Runtime requirement

Requires an OMP runtime supporting get_skill_diagnostics, set_skill_startup_diagnostics, get_state.skillDiagnostics, and skill_diagnostics_update from the linked PR. This does not change the existing CLI skill listing fallback.

@andrebrait
andrebrait force-pushed the feat/skill-startup-diagnostics branch from 98d7a05 to d5d6093 Compare October 4, 2026 02:20
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 4, 2026
Squashed from origin/feat/skill-startup-diagnostics (d5d6093) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 4, 2026
Squashed from origin/feat/skill-startup-diagnostics (773062c) onto b3556e7.

kahme247 commented Oct 4, 2026

Copy link
Copy Markdown
Owner

Not merged yet. main now carries #193, #194 and #195, and this branch conflicts with them in hooks/useAgentSession.ts, lib/rpc-manager.ts, their test files, lib/omp/settings-config.test.mjs and CHANGELOG.md. Could you rebase onto main and re-run the suite? It also depends on the omp change in can1357/oh-my-pi#14148, so I'd merge it once that is released, or earlier if you confirm older omp degrades safely as described.


Generated by Claude Code

Use OMP live resolution instead of duplicating discovery.
Keep the native notice preference and never resume stopped sessions for
inspection.
Add an icon-only dismiss button that hides the current report until its diagnostics change, and shorten the disable action to "Turn this off" so the notice text keeps its width on phones.
@andrebrait
andrebrait force-pushed the feat/skill-startup-diagnostics branch from 773062c to 9fc786c Compare October 5, 2026 00:29
@andrebrait

Copy link
Copy Markdown
Contributor Author

Integration rebase onto current upstream exposed a stale anthropicSlowMode state field and two cached conflict resolutions missing test closures. Removed the obsolete field (the provider-neutral slowMode fields are already retained) and restored the test boundaries without weakening assertions.

Verification: typecheck passed; RPC manager, native settings, and hook RPC suites passed all 214 tests. These repairs are on this PR branch so integration rebuilds retain them.

andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (38c2104) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (38c2104) onto upstream/main.
@andrebrait

Copy link
Copy Markdown
Contributor Author

Verification for head 38c21045ee82a9e213c12e8ca4bc25b4e5d15ecc, rebased onto main at c4f89b6:

  • The full local suite passed: 1,157 passed, 5 platform skips, 0 failures. Typechecking passed; lint completed with 0 errors and 11 existing warnings.
  • The unchanged head also passed the existing Verify PR workflow on both Ubuntu and Windows in my fork, including production builds and Windows tray install/status/uninstall: https://github.com/andrebrait/ompweb/actions/runs/37255129715.

The failed upstream Windows build matches the known Next.js Google-font URL parsing bug, vercel/next.js#99114. I reproduced the exact TypeError with the installed Next.js loader by supplying an extensionless font URL; the .woff2 URL control succeeds. This PR does not change the fonts or Next.js dependencies. Please rerun the failed Windows job here; my account has read-only repository permissions, so I cannot rerun it myself. No checks were disabled or assertions weakened.

Older OMP compatibility: I tested the actual PR route handlers and process wrapper with a real released, unpatched OMP 18.6.1, in isolated HOME/agent directories with no credentials or model calls. Its state omits skillDiagnostics. The diagnostics getter and setter return HTTP 400 Unknown command immediately; neither emits an error SSE nor replaces the child. get_state, get_messages, get_session_stats, and get_available_models continue working on the same session. With no live child, both diagnostics commands return 409 without starting OMP, including after stopping the owned child.

I also checked the browser against released 18.1.17: saved transcripts, settings, reload/reconnect, and ordinary non-model RPC remain usable. Manual diagnostics show the localized unavailable message, not a fabricated clean report. Caveat for that older release: its unknown-command replies omit request IDs, so inspection takes the existing five-second timeout and can show an Unknown command notice; the session still survives. Its startup-notices preference can be saved without preventing the old runtime from starting, but the diagnostics feature itself remains unavailable until OMP provides the new RPC support.

andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (38c2104) onto upstream/main.
@andrebrait

Copy link
Copy Markdown
Contributor Author

Shortened the startup warning action to Turn off and anchored the accessible 24px dismiss button in the warning’s top-right corner, outside the action-row flex flow. Browser-verified the actual component with compiled app CSS at 320, 768, 1024 and 1440px: no horizontal overflow; Details, dismissal and Turn off all work. Focused diagnostics tests (3), TypeScript and component ESLint pass. Condensed adversarial review: no findings.

andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (9dbf567) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (9dbf567) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (9dbf567) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (9dbf567) onto upstream/main.
@andrebrait

Copy link
Copy Markdown
Contributor Author

Reverted the warning × positioning to its original inline action-row layout, as requested. Kept Turn off unchanged. Actual component/CSS browser smoke at 320px and 1440px confirms inline placement without overflow; diagnostics tests (3), TypeScript and component ESLint passed. Condensed review: no findings.

andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (444e413) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (444e413) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (444e413) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (444e413) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (444e413) onto upstream/main.
andrebrait added a commit to andrebrait/ompweb that referenced this pull request Oct 5, 2026
Squashed from origin/feat/skill-startup-diagnostics (444e413) onto upstream/main.
kahme247 added a commit that referenced this pull request Oct 5, 2026
@kahme247
kahme247 merged commit 104322f into kahme247:main Oct 5, 2026
3 checks passed
@andrebrait
andrebrait deleted the feat/skill-startup-diagnostics branch October 6, 2026 10:18
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.

2 participants