feat(web-next): integrate Skill Builder IDE component#164
Merged
dgarson merged 5 commits intofeat/horizon-ui-completefrom Feb 24, 2026
Merged
feat(web-next): integrate Skill Builder IDE component#164dgarson merged 5 commits intofeat/horizon-ui-completefrom
dgarson merged 5 commits intofeat/horizon-ui-completefrom
Conversation
- Add zustand dependency for state management - Create commandRegistry store with add/remove/execute commands - Include default navigation and action commands - Supports NL Actions via keyword matching
- Add SkillBuilderEditor view that wraps the SkillBuilder component - Connect save and test handlers with basic validation - Add navigation entry and route for skill-builder - Allows creating and editing skills from the UI bs-ux-5-impl
dgarson
commented
Feb 24, 2026
Owner
Author
dgarson
left a comment
There was a problem hiding this comment.
Summary:
- Integrates Skill Builder into web-next navigation (new lazy-loaded
SkillBuilderEditorview and nav item). - Adds a command registry store scaffold with Zustand.
- Strengthens privacy handling in assistant error formatting by redacting common PII and suppressing long prompt/input echoes.
- Adds inline skill credential-disclosure behavior for non-owner senders and corresponding tests.
What I checked:
- Diff review across web-next integration and backend behavior changes.
- Security/privacy implications of new disclosure + redaction logic.
- Test coverage for newly added backend behaviors.
Validation run:
pnpm vitest src/agents/pi-embedded-helpers.formatassistanterrortext.test.ts src/auto-reply/reply/get-reply-inline-actions.credential-disclosure.test.ts --run- Result: passing.
Concerns / suggestions:
SkillBuilderEditorcurrently usesalert(...)andconsole.log(...)in save flow.
- This is understandable for scaffolding, but it may not be ideal UX in production paths.
- Suggestion: replace with non-blocking in-app toast/status pattern and a clearly marked TODO/API integration path.
commandRegistrynavigation actions usewindow.location.href = ....
- In SPA-style view switching, hard navigations can bypass app state and may land on routes that are not first-class router pages.
- Suggestion: align command actions with existing internal navigation model (same mechanism used by App view switching), or gate this store until wired.
Blocking issues before merge:
- None strictly blocking from this review, assuming the current Skill Builder save/test behavior is intentionally scaffold-level for this phase.
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
Integrates the Skill Builder IDE component into the web-next app, making it accessible from the navigation.
Changes
Features
The Skill Builder provides:
Testing
Related