Port Registrar: onboarding scaffolder to emit port-registrar-compliant templates#2368
Draft
TalZaccai wants to merge 2 commits into
Draft
Port Registrar: onboarding scaffolder to emit port-registrar-compliant templates#2368TalZaccai wants to merge 2 commits into
TalZaccai wants to merge 2 commits into
Conversation
Updates the scaffolder so newly-generated websocket-bridge and view-ui agents follow the modern port-allocation pattern: OS-assigned port + context.registerPort(role, port), discoverable by external clients via discoverPort(name, role). - buildWebSocketBridgeTemplate (scaffoldPlugin): static start(port=0) factory, close(), connected getter, sendCommand helper. - buildWebSocketBridgeHandler (scaffoldAgent): full AppAgent lifecycle with refcounted shared server, registerPort, and backstop close. - buildViewUiHandler: full AppAgent with view server, registerPort, setLocalHostPort for shell integration, and ActivityContext-driven openLocalView in executeAction. - Both templates honor <NAME>_BRIDGE_PORT / <NAME>_VIEW_PORT env-var overrides for debugging. - Updated PLUGIN_TEMPLATES nextSteps for websocket-bridge to reflect the new await <PascalName>Bridge.start() usage. - agent-patterns.md sections 5 and 8 document the new port contract. The office-addin template is left unchanged in this PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the onboarding scaffolder templates (websocket-bridge + view-ui) to follow the port-registrar contract: bind on OS-assigned ports by default and publish them via context.registerPort(...), with docs updated to describe the new contract.
Changes:
- WebSocket bridge template now uses an async
start(port=0)factory, tracks connections, and exposes.portfor registrar publication. - WebSocket bridge agent template now manages a refcounted shared server, registers the bound port per session, and supports env-var port overrides.
- View UI agent template now binds an HTTP server on
port=0, registers/discloses ports, and surfaces the view viaActivityContext.openLocalView.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts | Updates scaffolded templates and next-steps text to use dynamic port binding + port registration patterns. |
| ts/docs/architecture/agent-patterns.md | Documents the new register/discover port contract and env-var overrides for bridge + view patterns. |
Comments suppressed due to low confidence (1)
ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts:1
isFirstForSessionis computed before theawait ensureSharedBridge(). If twoupdateAgentContext(true, ...)calls interleave for the same session (e.g. two schemas enabled concurrently), the second call can observeenabledSchemas.size > 0and skip registration/refcounting, and then the first call can fail and roll back—leaving the session enabled with noportRegistrationand no refcount increment. To make this robust, determine “first for session” at the time of registration (after the await) or gate registration onctx.portRegistration(e.g., register iffctx.portRegistrationis undefined andctx.enabledSchemas.size > 0), ideally under a per-session async mutex to prevent interleaving.
// Copyright (c) Microsoft Corporation.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| defaultSubdir: "src", | ||
| nextSteps: | ||
| "Start the bridge with `new WebSocketBridge(port).start()` and connect your plugin.", | ||
| 'Bind on an OS-assigned port via `await ${PascalName}Bridge.start()`, then publish the bound `.port` from your handler with `context.registerPort("default", bridge.port)` so external clients can discover it.', |
Comment on lines
1564
to
1570
| export function instantiate(): AppAgent { | ||
| return { | ||
| initializeAgentContext, | ||
| updateAgentContext, | ||
| closeAgentContext, | ||
| executeAction, | ||
| }; | ||
| } |
Comment on lines
+1018
to
+1019
| server.on("error", () => { | ||
| /* TODO: log */ |
Comment on lines
+1034
to
+1042
| public close(): Promise<void> { | ||
| for (const c of this.clients.values()) { | ||
| if (c.readyState === WebSocket.OPEN) c.close(); | ||
| } | ||
| this.clients.clear(); | ||
| return new Promise((resolve) => | ||
| this.server.close(() => resolve()), | ||
| ); | ||
| } |
Comment on lines
+1995
to
+2001
| if (activityContext) { | ||
| // ActivityContext is attached so the shell can open the view. | ||
| // The shape comes from the SDK; cast through unknown to keep | ||
| // the template free of internal-only ActionResult fields. | ||
| (result as unknown as { activityContext: ActivityContext }).activityContext = | ||
| activityContext; | ||
| } |
Comment on lines
+191
to
+192
| per session; `closeAgentContext` is the backstop that releases the | ||
| registration and closes the server if disable wasn't called. |
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
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
Updates the onboarding scaffolder so newly-generated
websocket-bridgeandview-uiagents follow the modern port-allocation pattern introduced by the port-registrar work: OS-assigned port +context.registerPort(role, port), discoverable by external clients viadiscoverPort(name, role).Templates emitted by
pnpm onboarding scaffoldwere stuck on hardcoded ports (8081 / 9092) and a module-level singleton server, which collides with the dynamic-port contract every other agent has migrated to.What changed
ts/packages/onboarding/src/scaffolder/scaffolderHandler.ts:buildWebSocketBridgeTemplate(scaffoldPlugin): emits a class with a staticstart(port = 0)factory,close(),connectedgetter, andsendCommandhelper. WrapsAgentWebSocketServerfromwebsocket-utilsso callers never bind a literal port.buildWebSocketBridgeHandler(scaffoldAgent): fullAppAgentwithinitializeAgentContext/updateAgentContextlifecycle, refcounted shared server,context.registerPort("bridge", port), and a backstopclose().buildViewUiHandler: fullAppAgentwith view server,context.registerPort("view", port),setLocalHostPortfor shell integration, and anActivityContext-drivenopenLocalViewinexecuteAction.<NAME>_BRIDGE_PORT/<NAME>_VIEW_PORTenv-var overrides for debugging.PLUGIN_TEMPLATES.nextStepsupdated to show the newawait <PascalName>Bridge.start()usage.ts/docs/architecture/agent-patterns.md:"default", env-var overrides).Validation
pnpm --filter onboarding build✓prettier --check✓repo-policy-check✓websocket-bridgeagent and aview-uiagent in a temp dir; both build clean, register their ports with the dispatcher on init, and are discoverable viadiscoverPort. Generated code matches the patterns established in already-migrated agents (e.g.code,localView).Related