Skip to content

Port Registrar: Adding @system ports command#2358

Open
TalZaccai wants to merge 10 commits into
mainfrom
talzacc/system-ports-command
Open

Port Registrar: Adding @system ports command#2358
TalZaccai wants to merge 10 commits into
mainfrom
talzacc/system-ports-command

Conversation

@TalZaccai
Copy link
Copy Markdown
Contributor

Depends on PR #2345. Branched off talzacc/browser-agent-migration and must merge after it.

Goal

A debug/introspection command that lists every live entry in the PortRegistrar plus the current client count for that entry, formatted to mirror @config agent (emoji + HTML table in the shell, chalk fixed-width in the CLI).

Example shell output:

Agent Role Port Clients
🌐 browser default 54231 2
💻 code default 54232 1

Design (revised after rubber-duck critique)

Push model for client counts (mirrors notifyReadinessChanged)

  1. SDKSessionContext.notifyClientCountChanged(role, count). Agent name inferred from session context (same as notifyReadinessChanged and registerPort). Best-effort: errors swallowed but logged under typeagent:dispatcher:clientCount:warn.
  2. agent-rpc — plumb through types.ts / server.ts / client.ts the same way notifyReadinessChanged is.
  3. Count store lives inside PortRegistrar (not AppAgentManager). In agent-server mode the registrar is shared across CommandHandlerContexts but AppAgentManager is per-context — co-locating fixes the scope mismatch. New API:
    • setClientCount(agentName, role, sessionContextId, count) — ignores writes for which no live allocation exists (defends against late notifications arriving after releaseAllForSession).
    • getClientCount(agentName, role, sessionContextId): number | undefinedundefined means "never reported", distinct from 0.
    • On release / releaseAllForSession, drop matching count entries.
  4. Agents — call sessionContext.notifyClientCountChanged(role, count) from a new onClientCountChanged(count) WS-server callback, invoked after the clients map mutation in both connect + disconnect paths (fixes the off-by-one in today's pre-mutation onClientDisconnected).

Code agent caveat (no session identity)

CodeAgentWebSocketServer has a single global clients: Map<string, WebSocket> and no session keying. Per-session client counts are impossible without changing the code-WS protocol (out of scope). Acceptable because: at render time we group rows by (agentName, role, port) and sum counts; for code that collapses to one row reflecting the global count, no matter how many sessions registered the same shared port.

Command handler

@system ports in dispatcher/src/context/system/handlers/portsCommandHandler.ts. Iterates portRegistrar.list(), groups by (agentName, role, port), looks up emoji via agents.getAppAgentEmoji(name), joins client count via the registrar. Sort: alphabetical by (agentName, role), then by port ascending.

Render:

  • HTML for shell (displayResult with same table styling as formatAgentStatusHtml). All cell text HTML-escaped (agent name and role are agent-defined free text).
  • CLI plain text via chalk, fixed-width columns, mirrors the chalk path in showAgentStatus.
  • "(no ports registered)" warning if list is empty.
  • :system rows (sessionContextId === SYSTEM_SESSION_CONTEXT_ID) get a [system] suffix on the agent column so users can spot self-registered infra (agent-server, standalone shell discovery WS).
  • Client count column: integer if getClientCount returned a value, ? if undefined (agent never reported).

@TalZaccai TalZaccai requested a review from Copilot May 19, 2026 00:45
@TalZaccai TalZaccai changed the title Talzacc/system ports command Adding @system ports command May 19, 2026
@TalZaccai TalZaccai changed the title Adding @system ports command Adding @system ports command May 19, 2026
@TalZaccai TalZaccai changed the title Adding @system ports command Port Registrar: Adding @system ports command May 19, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a new diagnostics command to enumerate all live PortRegistrar allocations (plus connected-client counts where published), and plumbs “push” notifications from agents up through the SDK/agent-rpc/dispatcher so readiness and client-count information can be refreshed from event-driven code paths. It also extends the standalone Electron shell to host an in-process discovery WebSocket (mirroring agent-server’s discovery channel) so external clients like the browser extension can locate dynamically assigned in-process agent ports.

Changes:

  • Add @ports command (HTML table for shell + fixed-width CLI table) to list registered ports and client counts.
  • Add SDK + RPC + dispatcher support for SessionContext.notifyReadinessChanged() and SessionContext.notifyClientCountChanged(...), with PortRegistrar storing per-session client counts.
  • Add discovery-channel handler factory in agent-server-protocol, plus standalone shell discovery server and browser/code agent wiring (Origin gating + client-count push).

Reviewed changes

Copilot reviewed 42 out of 43 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
ts/pnpm-lock.yaml Updates workspace dependency graph for new/updated packages used by shell/dispatcher/agents.
ts/packages/shell/src/main/instance.ts Creates a shared PortRegistrar and (best-effort) starts a standalone discovery WS on the default agent-server port in local mode.
ts/packages/shell/src/main/discoveryServer.ts New: standalone shell discovery WebSocket server hosting the agent-server discovery RPC channel.
ts/packages/shell/src/main/browserIpc.ts Switches inline-browser WS connection to discovery-then-connect (no hardcoded browser-agent port).
ts/packages/shell/README.md Documents local-mode discovery WS behavior and failure mode when port 8999 is busy.
ts/packages/shell/package.json Adds dependencies needed for in-process discovery WS and protocol handlers.
ts/packages/dispatcher/dispatcher/test/sessionContext.spec.ts Adds tests for best-effort readiness/client-count notifications on SessionContext.
ts/packages/dispatcher/dispatcher/test/portRegistrar.spec.ts Adds tests for per-session client-count storage and cleanup behavior in PortRegistrar.
ts/packages/dispatcher/dispatcher/src/execute/sessionContext.ts Implements notifyReadinessChanged and notifyClientCountChanged in dispatcher session contexts.
ts/packages/dispatcher/dispatcher/src/context/system/systemAgent.ts Registers the new ports command in the top-level system command table.
ts/packages/dispatcher/dispatcher/src/context/system/handlers/portsCommandHandler.ts New: implements @ports diagnostic command with HTML + CLI renderers.
ts/packages/dispatcher/dispatcher/src/context/system/handlers/configCommandHandlers.ts Removes the old @config ports handler (superseded by @ports).
ts/packages/dispatcher/dispatcher/src/context/portRegistrar.ts Adds client-count storage APIs (setClientCount/getClientCount) and clears counts on release.
ts/packages/dispatcher/dispatcher/README.md Documents the new @ports diagnostic command.
ts/packages/agentServer/server/src/server.ts Switches discovery channel wiring to use shared createDiscoveryHandlers factory.
ts/packages/agentServer/protocol/src/protocol.ts Adds createDiscoveryHandlers(...) factory to share discovery RPC behavior across hosts.
ts/packages/agentServer/protocol/src/index.ts Re-exports createDiscoveryHandlers.
ts/packages/agentServer/protocol/README.md Documents the discovery channel and the shared handler factory.
ts/packages/agentSdk/src/agentInterface.ts Adds notifyReadinessChanged() and notifyClientCountChanged(...) to the SessionContext SDK interface.
ts/packages/agents/code/test/codeUpdateContext.spec.ts Updates test stubs to include notifyClientCountChanged.
ts/packages/agents/code/src/originAllowlist.ts New: code-agent-specific Origin allowlist helper.
ts/packages/agents/code/src/codeAgentWebSocketServer.ts Adds Origin gating via verifyClient and emits post-mutation client-count updates.
ts/packages/agents/code/src/codeActionHandler.ts Fans out client-count changes to active sessions and publishes initial count on enable.
ts/packages/agents/browser/test/unit/agentWebSocketServer.test.ts Updates tests for async start/close, Origin gating, and close() closing tracked clients.
ts/packages/agents/browser/test/serviceWorker/websocket.test.ts Updates tests for discovery-based endpoint resolution and reconnect-interval singleton behavior.
ts/packages/agents/browser/src/extension/views/options.ts Renames setting to agentServerHost and allows blank input to mean “use default”.
ts/packages/agents/browser/src/extension/views/options.html Updates options UI labels/help text for agent-server URL + automatic port discovery.
ts/packages/agents/browser/src/extension/serviceWorker/websocket.ts Uses discovery channel to resolve browser-agent port; removes settings cache; fixes reconnect timer leak; adds singleton guard.
ts/packages/agents/browser/src/extension/serviceWorker/types.ts Renames settings field to agentServerHost.
ts/packages/agents/browser/src/extension/serviceWorker/storage.ts Adjusts persisted settings retrieval for agentServerHost and clarifies semantics in docs.
ts/packages/agents/browser/src/extension/serviceWorker/index.ts Updates storage change handling and hardens web page message parsing.
ts/packages/agents/browser/src/agent/websiteMemory.mts Updates mock session context to satisfy new SessionContext methods.
ts/packages/agents/browser/src/agent/readiness.mts Updates readiness documentation to reflect dynamic port assignment and env override.
ts/packages/agents/browser/src/agent/originAllowlist.mts New: browser-agent-specific Origin allowlist helper.
ts/packages/agents/browser/src/agent/browserActions.mts Adds per-session port registration + enable/disable tracking fields.
ts/packages/agents/browser/src/agent/browserActionHandler.mts Refactors shared WS server lifecycle (async start/close, refcount, cleanup), registers port per session, and pushes readiness/client-count changes.
ts/packages/agents/browser/src/agent/agentWebSocketServer.mts Adds async start(), Origin gating, off-by-one-safe client-count callbacks, and an async close() that closes tracked clients first.
ts/packages/agents/browser/README.md Documents dynamic-port + discovery flow and the BROWSER_WEBSOCKET_PORT override.
ts/packages/agents/browser/package.json Adds dependency needed for discovery client usage.
ts/packages/agentRpc/src/types.ts Adds RPC surface for notifyReadinessChanged and notifyClientCountChanged.
ts/packages/agentRpc/src/server.ts Plumbs the new SessionContext notification calls through agent-rpc server helpers.
ts/packages/agentRpc/src/client.ts Dispatches incoming RPC calls to the new SessionContext notification methods.
ts/docs/architecture/dispatcher.md Documents the new @ports command in the dispatcher architecture overview.
Files not reviewed (1)
  • ts/pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread ts/packages/dispatcher/dispatcher/src/context/portRegistrar.ts
Comment thread ts/packages/agentSdk/src/agentInterface.ts
Comment thread ts/packages/shell/src/main/browserIpc.ts
Comment thread ts/packages/agents/browser/src/agent/agentWebSocketServer.mts
Comment thread ts/packages/dispatcher/dispatcher/src/execute/sessionContext.ts
Comment thread ts/packages/agents/browser/src/agent/originAllowlist.mts
Comment thread ts/packages/agents/code/src/originAllowlist.ts
TalZaccai added a commit that referenced this pull request May 19, 2026
- portsCommandHandler: sum per-session client counts (treating
  undefined as 0) instead of Math.max. Browser agent reports
  genuinely per-session counts; max undercounts when multiple
  sessions each have clients.
- codeActionHandler: attribute the (global) shared-server count to
  a single primary session (first in insertion order); other
  sessions report 0. Prevents double-counting now that the handler
  sums. Transfers ownership when the primary disables.
- agentWebSocketServer (browser): coerce 
eq.headers.origin
  array values to a single string before passing to the allowlist
  (Node typings allow string | string[] | undefined).
- browserIpc: use bracketed hostname when reconstructing the
  inline-browser WS URL so IPv6 literals like ::1 round-trip as
  ws://[::1]:port/... instead of the invalid ws://::1:port/...
- originAllowlist (browser + code): also accept unbracketed ::1
  for robustness against URL parser/serializer differences across
  runtimes (precedent: examples/workflow/engine/builtinTasks.ts).

Doc-nit suggestions (rename @System ports -> @ports) were skipped:
ports is registered inside systemHandlers so @System ports
is the correct invocation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@TalZaccai TalZaccai mentioned this pull request May 19, 2026
TalZaccai and others added 9 commits May 19, 2026 13:07
Upgrade the existing `@config ports` command to surface
PortRegistrar allocations directly (instead of the legacy
`getLocalHostPort` shim) and add a Clients column that mirrors the
emoji-rich formatting used by `@config agent`. Rows are grouped by
(agent, role, port) and `:system` rows are tagged `[system]`.
Empty registrar renders `No ports registered`.

To drive the new column, add `SessionContext.notifyClientCountChanged(role, count)`
to the agent SDK following the existing `notifyReadinessChanged`
pattern. The implementation delegates to a new
`PortRegistrar.setClientCount` / `getClientCount` pair scoped by
`(agentName, role, sessionContextId)`. Writes with no live allocation
are silently dropped (defends against late notifications after
`releaseAllForSession`); `release` and `releaseAllForSession`
both drop matching count entries. `getClientCount` returns
`undefined` for `never reported` so the render can distinguish it
from an explicit `0`.

Wire the SDK method through:
  - agentRpc: caller + callee for `notifyClientCountChanged`.
  - browser agent: `AgentWebSocketServer` gains an
    `onClientCountChanged` callback that fires AFTER every
    sessionMap mutation (fixes an off-by-one had we reused the
    existing `onClientDisconnected` hook which fires pre-delete).
  - code agent: `CodeAgentWebSocketServer` exposes
    `onClientCountChanged`; `codeActionHandler` maintains a
    module-level `sharedActiveSessions` set so the single shared
    server fans the same global count out to every active session.

Tests:
  - portRegistrar.spec: setClientCount round-trip, drop-on-no-allocation,
    accepts 0, rejects negative + non-integer, release + releaseAllForSession
    drop counts, per-session isolation.
  - sessionContext.spec: notifyClientCountChanged delegates with agent
    name + sessionContextId; swallows setClientCount errors.
  - codeUpdateContext.spec: stub SessionContext gets the new method.

Build green for agent-dispatcher, browser-typeagent, code-agent.
Tests: dispatcher 681/681, browser 255/255, code 18/18. Prettier +
repo-policy-check clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The previous commit upgraded the existing @config ports stub. Per
discussion, the right home for this is the top-level @System
namespace alongside the other diagnostic commands (@System session,
@System history, @System env, etc.) since it's a system-wide
diagnostic, not a configuration toggle.

  - Extract the handler into its own file
    system/handlers/portsCommandHandler.ts exporting
    PortsCommandHandler (the prior commit's class).
  - Register it under systemHandlers in systemAgent.ts.
  - Remove the now-empty 'ports' entry from getConfigCommandHandlers.
  - Update agentServer/protocol/README.md to mention @System ports
    (not @config ports).

Build clean for agent-dispatcher; tests still 681/681.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The registrar contains an entry for the agent-server's own listening
port registered under the well-known 'agent-server' name (see
server.ts:464). That name is not a real app-agent, so calling
agents.getAppAgentEmoji('agent-server') throws 'Unknown app agent:
agent-server' and the whole @System ports command bombs out — only
visible when the dispatcher is running inside the agent-server
(detached mode), since standalone shell mode has no such entry.

Wrap the emoji lookup in a try/catch fallback to an empty string so
synthetic / non-app-agent entries render with no emoji instead of
killing the command.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Multiple conversation sessions registered to the SAME physical port
(shared servers like browser and code) all observe the same global
connected-client count and each fans it out independently. Summing
them double-counts: 2 sessions x 2 clients reported = 4. Taking the
max within a (agent, role, port) group gives the true count for
shared servers and remains correct for per-session servers (where
each group has exactly one contributor).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…h counts

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an entry for @ports to docs/architecture/dispatcher.md's
system-agent command list and to dispatcher/README.md under a new
Diagnostics section. Both call out the (agent, role, port) grouping,
the agent-server's own listen-port row, and the N/A semantic for
agents that don't publish client counts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- portsCommandHandler: sum per-session client counts (treating
  undefined as 0) instead of Math.max. Browser agent reports
  genuinely per-session counts; max undercounts when multiple
  sessions each have clients.
- codeActionHandler: attribute the (global) shared-server count to
  a single primary session (first in insertion order); other
  sessions report 0. Prevents double-counting now that the handler
  sums. Transfers ownership when the primary disables.
- agentWebSocketServer (browser): coerce 
eq.headers.origin
  array values to a single string before passing to the allowlist
  (Node typings allow string | string[] | undefined).
- browserIpc: use bracketed hostname when reconstructing the
  inline-browser WS URL so IPv6 literals like ::1 round-trip as
  ws://[::1]:port/... instead of the invalid ws://::1:port/...
- originAllowlist (browser + code): also accept unbracketed ::1
  for robustness against URL parser/serializer differences across
  runtimes (precedent: examples/workflow/engine/builtinTasks.ts).

Doc-nit suggestions (rename @System ports -> @ports) were skipped:
ports is registered inside systemHandlers so @System ports
is the correct invocation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@TalZaccai TalZaccai force-pushed the talzacc/system-ports-command branch from 2f1489c to 637bea0 Compare May 19, 2026 20:12
@TalZaccai TalZaccai requested a review from Copilot May 19, 2026 20:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.

Comment thread ts/packages/dispatcher/dispatcher/README.md Outdated
Comment thread ts/packages/agentSdk/src/agentInterface.ts
Comment thread ts/packages/shell/src/main/browserIpc.ts Outdated
Comment thread ts/packages/agents/browser/src/agent/originAllowlist.mts
- portsCommandHandler: escape emoji in HTML output (XSS hardening; agent-provided metadata was interpolated raw)
- dispatcher README: rename @ports -> @System ports (correct invocation)
- browserIpc: clarify the IPv6 bracket comment to match the actual hostname-based implementation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@TalZaccai TalZaccai temporarily deployed to development-fork May 19, 2026 20:38 — with GitHub Actions Inactive
@TalZaccai TalZaccai temporarily deployed to development-fork May 19, 2026 20:38 — with GitHub Actions Inactive
@TalZaccai TalZaccai marked this pull request as ready for review May 19, 2026 22:45
@TalZaccai TalZaccai requested a review from robgruen May 19, 2026 22:45
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