feat(cli): MCP list_peers + runner hub auth for peer discovery - #1372
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
- [Major] Preserve default-hub auto-start after exporting child credentials —
initializeToken()now callsexportHapiHubAuthEnv(), which always writes the resolved defaulthttp://localhost:3006intoprocess.env.HAPI_API_URL. Every agent command callsmaybeAutoStartServer()immediately afterward, and its existing guard treats any value in that env var as an explicit override, so installations relying on the bundled hub no longer auto-start it and proceed to connection failure. Evidence:cli/src/ui/tokenInit.ts:30; related guardcli/src/utils/autoStartServer.ts:94.
Suggested fix:// Capture whether the user supplied the URL before initializeToken() mirrors it. const apiUrlWasExplicitlyConfigured = Boolean(process.env.HAPI_API_URL) async function shouldAutoStartServer(): Promise<boolean> { if (apiUrlWasExplicitlyConfigured) { return false } // existing settings/default checks... }
Summary
Review mode: initial
One Major regression found. The new MCP/list formatting paths did not produce another finding above the confidence threshold.
Testing
Not run (automation): bun is unavailable in this runner (/bin/bash: bun: command not found). Add a regression test covering initializeToken() followed by maybeAutoStartServer() with no explicit API URL and a saved local token.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Keep settings-backed CLI_API_TOKEN out of the wrapped agent environment — the new assignment copies the hub's shared secret from settings/prompt-backed configuration into
process.env, and all wrapped agent launchers inherit that environment. The secret can be exchanged for a namespace JWT covering the hub API, bypassing the deliberate approval boundary aroundinspect_peerandping_peer. Evidencecli/src/agent/hapiSessionEnv.ts:64; related contextcli/src/agent/runAgentPty.ts:160,hub/src/web/routes/auth.ts:28.
Suggested fix:
export function exportHapiHubAuthEnv(options: ExportHapiHubAuthEnvOptions = {}): void {
if (options.exportApiUrl) {
const apiUrl = configuration.apiUrl?.trim()
if (apiUrl) process.env.HAPI_API_URL = apiUrl
}
// Do not mirror configuration.cliApiToken into process.env.
// Settings/prompt-backed helper CLIs can reread settings; env-backed tokens are already inherited.
}
- [Minor] Exclude the calling session from `list_peers` — `/api/sessions` returns every session in the namespace, and this new path formats that result without removing `client.sessionId`. The caller is normally active and recently updated, so it is likely to appear first and can be mistaken for a peer, causing `inspect_peer` or `ping_peer` to self-target. Evidence `cli/src/claude/utils/startHappyServer.ts:289`; related context `hub/src/web/routes/sessions.ts:79`.
Suggested fix:
```ts
const sessions = (await listPeerSessions({ limit: limit + 1 }))
.filter((session) => session.id !== client.sessionId)
.slice(0, limit)
Summary
Review mode: follow-up after new commits.
The previous auto-start finding is no longer present. One Major security issue and one Minor correctness issue remain.
Testing
Not run (automation): PR code was not executed. Add coverage proving settings-backed tokens are absent from spawned agent environments and list_peers excludes the current session while respecting limit.
HAPI Bot
|
Addressed in follow-up push:
|
There was a problem hiding this comment.
Findings
- [Major] Use HAPI's effective session title and normalize it to one line — the new formatter reads only
metadata.name, but normal session bootstrap leaves that field unset and agent-generated titles are persisted inmetadata.summary.text. As a result, most discoverable sessions appear as(unnamed), which defeats title-based peer discovery. Raw whitespace also lets a title forge extra output rows. Evidencecli/src/modules/pingPeer/pingPeer.ts:394; related contextcli/src/api/apiSession.ts:834,web/src/lib/sessionTitle.ts:10.
Suggested fix:
metadata?: {
name?: string
summary?: { text: string }
// existing fields...
}
const rawName = session.metadata?.name
?? session.metadata?.summary?.text
?? session.metadata?.path?.split('/').filter(Boolean).pop()
?? session.id.slice(0, 8)
const name = rawName.replace(/\s+/g, ' ').trim().slice(0, 255)
**Summary**
Review mode: follow-up after new commits.
One Major correctness issue remains. The earlier token-exposure and self-listing findings are no longer present at the current head.
**Testing**
Not run (automation): PR code was not executed. Add formatter coverage for a summary-only session and a title containing newlines/tabs.
*HAPI Bot*
|
Addressed: |
There was a problem hiding this comment.
Findings
- [Minor] Return resolvable peer identifiers — the shortlist emits only eight characters, but the resolver rejects ambiguous prefixes. Two sessions sharing that prefix become indistinguishable and neither can be selected from
list_peers. Evidencecli/src/modules/pingPeer/pingPeer.ts:417; related contextcli/src/modules/pingPeer/pingPeer.ts:169.
Suggested fix:return ` ${session.id} active=${session.active} flavor=${flavor} ${name}`
- [Minor] Enforce the discovery limit at the hub — the new auto-approved tool passes a limit, but
GET /api/sessionsignores it and computes/serializes every session summary, including scheduled-count lookups, before the client discards all but 30–100 rows. Repeated agent calls therefore remain unbounded by the advertised limit. Evidencecli/src/claude/utils/startHappyServer.ts:289; related contexthub/src/web/routes/sessions.ts:79.
Suggested fix:// Add a lightweight, server-limited peer query. const limit = Math.min(101, Math.max(1, Number(c.req.query('limit')) || 31)) const sessions = engine.getSessionsByNamespace(c.get('namespace')) .sort((a, b) => b.updatedAt - a.updatedAt) .slice(0, limit) .map(({ id, active, updatedAt, metadata }) => ({ id, active, updatedAt, metadata }))
Summary
Review mode: follow-up after new commits.
Two Minor correctness/performance issues remain.
Testing
Not run (automation): PR code was not executed; CI checks were pending at review time. Add a formatter test with colliding eight-character prefixes and a route-level test proving the peer limit is applied before summary/count work.
HAPI Bot
|
Addressed remaining Minor threads in e22c7f6:
Please re-run review on the tip. |
There was a problem hiding this comment.
Findings
- [Major] Preserve full-session resolution for ping/inspect - the new server-side slice makes the existing default
limit=500effective for everylistSessionscaller. BothpingPeerandinspectPeercall it without an override, so a cited full UUID outside the first 500 active-first rows now returnsnot_foundeven though the route previously returned all namespace sessions. Evidencehub/src/web/routes/sessions.ts:101; related contextcli/src/modules/pingPeer/pingPeer.ts:195,cli/src/modules/pingPeer/pingPeer.ts:449,cli/src/modules/pingPeer/pingPeer.ts:644.
Suggested fix:Keep the bounded argument only inasync function listSessions( apiUrl: string, jwt: string, http: AxiosInstance, limit?: number ): Promise<PingPeerSessionSummary[]> { // ... params: limit === undefined ? undefined : { limit }, }
listPeerSessions; let prefix/full-ID resolution omit it. - [Minor] Apply newest-first ordering before discovery truncation -
list_peerspromises newestupdatedAtrows, but it requests a limited response from an endpoint that sorts active sessions first and slices before the formatter re-sorts. With enough active sessions, a newer inactive peer is never returned. Evidencecli/src/claude/utils/startHappyServer.ts:289; related contexthub/src/web/routes/sessions.ts:85.
Suggested fix:Add the query option and have the hub applyconst sessions = await listPeerSessions({ limit: Math.min(101, limit + 1), order: 'updatedAt' })
updatedAtordering beforeslice. - [Minor] Strip Windows separators from fallback labels - splitting only on
/means a nameless Windows session emits its full absolute path as the auto-approvedlist_peerslabel instead of the documented basename. Evidencecli/src/modules/pingPeer/pingPeer.ts:386.
Suggested fix:const pathLabel = meta?.path?.split(/[\\/]/).filter(Boolean).pop()?.trim() const raw = meta?.name?.trim() || meta?.summary?.text?.trim() || pathLabel || session.id.slice(0, 8)
Summary
Review mode: follow-up after new commits.
Three findings remain: one Major resolver regression and two Minor discovery correctness/privacy issues.
Testing
Not run (automation): PR code was not executed. GitHub test was pending at review time. Add coverage for resolving a session beyond row 500, a newer inactive peer beyond an active-first limit, and a Windows path fallback.
HAPI Bot
|
Addressed tip review findings:
Tip: this commit. |
There was a problem hiding this comment.
Findings
- [Minor] Auto-approve the ACP title form — ACP permission derivation prefers the registered tool title, but the new allowlist only includes identifier aliases. Default-mode ACP sessions can therefore prompt or block on peer discovery. Evidence cli/src/modules/common/permission/BasePermissionHandler.ts:38; related context cli/src/agent/utils.ts:24, cli/src/claude/utils/startHappyServer.ts:283.
Suggested fix:"list peer sessions",
Summary
Review mode: follow-up after new commits.
One Minor finding remains. The three findings from the prior bot review no longer reproduce in the current diff.
Testing
Not run locally (automation): PR code was not executed. GitHub test passed. Add the registered title (List Peer Sessions) to the auto-approval test matrix.
HAPI Bot
|
Addressed Minor: auto-approve ACP title form |
There was a problem hiding this comment.
Findings
- [Minor] Preserve overflow after excluding the caller —
list_peersfetches onlylimit + 1rows and caps that fetch at 100, then removes the current session. If the caller occupies the prefetched row, the result can contain fewer than the requested number of peers and omit the “more” marker even when additional sessions exist; atlimit=100, it can return only 99 peers. The formatter’s numeric omitted count is also only the count inside this bounded sample, not the namespace total. Evidencecli/src/claude/utils/startHappyServer.ts:289; related contextcli/src/modules/pingPeer/pingPeer.ts:433.
Suggested fix:const sessions = await listPeerSessions({ limit: limit + 2 }) const peers = sessions.filter((session) => session.id !== client.sessionId) const hasMore = peers.length > limit text: formatPeerSessionsList(peers, { maxRows: limit, hasMore }) // In the formatter: if (hasMore) rows.push(' … more sessions available')
- [Minor] Separate URL and token recovery instructions — the new hint says configuring both
HAPI_API_URLandCLI_API_TOKENcan be replaced byhapi auth login, but that command only savescliApiToken; it never sets the remote hub URL. On a split-hub runner that is still using localhost, following the advertised recovery leaves the command pointed at the wrong host. Evidencecli/src/modules/pingPeer/pingPeer.ts:87; related contextcli/src/commands/auth.ts:61.
Suggested fix:const AUTH_RECOVERY_HINT = 'On a remote runner, set HAPI_API_URL to the runner hub, and set CLI_API_TOKEN ' + 'or run `hapi auth login` to save the token. Inside a HAPI session prefer MCP ...'
Summary
Review mode: follow-up after new commits.
Two Minor findings remain. The prior HAPI Bot finding about the ACP title form is fixed in the latest commits.
Testing
Not run (automation/security constraint: PR code was not executed). GitHub test and pr-review checks were pending at review time. Add coverage for a full limit=100 request with the caller in the result and for the no-overflow-marker case when more peers exist beyond the bounded fetch.
HAPI Bot
|
Addressed tip Minors:
|
There was a problem hiding this comment.
Findings
- [Minor] Avoid an exact overflow count from a truncated shell shortlist —
listPeerSessions()now fetches at most 200 rows, whileformatPeerSessionsList()derives the… N morecount only from that bounded sample. With more than 200 sessions,hapi ping-peer --listreports… 170 moreafter showing 30 even though the namespace contains additional sessions, which makes the new discovery output misleading. Evidencecli/src/commands/pingPeer.ts:141; related contextcli/src/modules/pingPeer/pingPeer.ts:377andcli/src/modules/pingPeer/pingPeer.ts:448.
Suggested fix:const maxRows = 30 const sessions = await listPeerSessions({ limit: peerListFetchLimit(maxRows) }) console.log(formatPeerSessionsList(sessions, { maxRows, hasMore: sessions.length > maxRows }))
Summary
Review mode: follow-up after new commits.
One Minor finding remains. The two findings from the prior HAPI Bot review are addressed by the latest commits.
Testing
Not run (automation/security constraint: PR code was not executed). git diff --check passed. GitHub test and pr-review checks were pending at review time. Add command-level coverage for a namespace larger than the bounded fetch and assert that --list uses a non-numeric overflow marker.
HAPI Bot
|
Addressed Minor: |
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits.
No reportable issues found in the latest full PR diff. The prior HAPI Bot Minor about reporting an exact overflow count from a bounded peer fetch is addressed by the new bounded fetch plus boolean overflow marker.
Residual risk: the live runner → MCP bridge → remote hub peer-discovery path was not exercised in this review.
Testing
Not run (automation/security constraint: PR code was not executed). git diff --check passed. GitHub test and pr-review checks were pending at review time.
HAPI Bot
4774315 to
3a03f36
Compare
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits.
No reportable issues found in the latest full PR diff. The only change since the prior HAPI Bot pass guards the inactivity tick against incomplete machineCache test doubles and adds the missing stubs; it does not introduce a production behavior regression above the reporting threshold.
Residual risk: the live runner to MCP bridge to remote hub peer-discovery path was not exercised in this review.
Testing
Not run (automation/security constraint: PR code was not executed). git diff --check passed. GitHub test and pr-review checks were pending at review time.
HAPI Bot
3a03f36 to
85f6fff
Compare
There was a problem hiding this comment.
Findings
- None.
Summary
Review mode: follow-up after new commits.
No reportable issues found in the latest full PR diff. The previous reviewed commit and current head have diverged, so the current 29-file diff was reviewed independently; the prior review and compare diff were used only as follow-up context.
Residual risk: the live remote-runner → MCP bridge → remote-hub peer-discovery path was not exercised in this review.
Testing
Not run (automation/security constraint: contributor code was not executed). git diff --check passed for the PR head. GitHub test and pr-review checks were pending at review time.
HAPI Bot
Runner-spawned agents could not discover same-hub peers without sitting on the hub host or pasting a session id. Add MCP list_peers (in-process credentials), export HAPI_API_URL/CLI_API_TOKEN after auth init for shell fallbacks, and clearer auth failure hints. Co-authored-by: Cursor <cursoragent@cursor.com>
exportHapiHubAuthEnv was writing the implicit localhost default into process.env, which made maybeAutoStartServer skip starting the bundled hub. Only export HAPI_API_URL when the URL came from env or settings; always still export CLI_API_TOKEN. Also fill missing deliveryMode on abort restore so web typecheck matches RawSendError (main tip unblock). Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Keep settings/prompt-backed hub secrets out of wrapped agent env so shell JWT+curl cannot bypass peer-tool approval. Fresh hapi re-reads settings; env-backed tokens already inherit. list_peers omits the calling session from the shortlist. Co-authored-by: Cursor <cursoragent@cursor.com>
list_peers was showing (unnamed) for ordinary sessions because titles live in metadata.summary.text. Match web getSessionTitle and collapse whitespace so each peer stays one agent-readable line. Co-authored-by: Cursor <cursoragent@cursor.com>
Short 8-char prefixes collide across UUID namespaces; print full ids so resolveSessionByPrefix stays unambiguous. Honor optional limit after sort so listPeerSessions stops loading the whole namespace for scheduled counts. Co-authored-by: Cursor <cursoragent@cursor.com>
CI tsc rejected Map<string, null> for getNextScheduledAtBySessionIds. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep GET /sessions?limit only for discovery callers. ping/inspect omit limit so full UUIDs outside the first 500 stay resolvable. Peer lists pass order=updatedAt so truncation matches newest-first. Basename fallback splits Windows paths. Co-authored-by: Cursor <cursoragent@cursor.com>
Permission derivation prefers request.title; match the MCP tool title form so default-mode ACP sessions do not prompt on discovery. Co-authored-by: Cursor <cursoragent@cursor.com>
Fetch limit+2 when excluding the caller so overflow still surfaces at limit=100. Clarify that auth login only saves the token, not HAPI_API_URL. Co-authored-by: Cursor <cursoragent@cursor.com>
Match MCP list_peers: fetch limit+1 and mark hasMore instead of claiming an exact omitted count from a 200-row sample. Co-authored-by: Cursor <cursoragent@cursor.com>
CI flake: 5s inactivity tick hit test doubles that only stubbed getOnlineMachinesByNamespace. Optional-call + stub the method. Co-authored-by: Cursor <cursoragent@cursor.com>
85f6fff to
b5b3489
Compare
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits.
No reportable issues found after reviewing the full latest PR diff. The prior HAPI Bot review and the intervening commit comparison were used only as follow-up context.
Residual risk: the split-hub/remote-runner MCP list_peers path and shell fallback were not exercised end-to-end; coverage remains unit/mock based.
Testing
Not run (automation/security constraint: contributor code was not executed). git diff --check passed for the reviewed head.
HAPI Bot
2 participants
Summary
list_peers(HTTP MCP + stdio bridge + flavor tool lists) so runner-spawned agents can discover same-hub peers without shellinghapior sitting on the hub host.initializeToken(), exportHAPI_API_URL/CLI_API_TOKENintoprocess.envso agent Shell fallbacks inherit the session hub credentials (web terminal PTYs still strip them).--list/ JWT exchange pointing athapi auth loginand MCPlist_peers.list_peersbefore inspect/ping when no citation is available.Test plan
bun x vitest runon pingPeer, hapiSessionEnv, BasePermissionHandler, startHappyServer, happyMcpStdioBridge, buildHapiMcpBridge, runAgentSession, tokenInit, pingPeer command suitesbun typecheckat repo rootlist_peersreturns peers;inspect_peer/ping_peerwith a listed idhapi ping-peer --listworks on runner host with matching hub authIssues
Fixes #1371