Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 43 additions & 2 deletions cli/src/agy/headless/agyHeadlessDriver.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { describe, expect, it, vi } from 'vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { EventEmitter } from 'node:events';
import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
import { logger } from '@/ui/logger';
import { buildAgyHeadlessArgs, AgyHeadlessDriver } from './agyHeadlessDriver';
import { buildAgyHeadlessArgs, buildAgySpawnEnv, AgyHeadlessDriver } from './agyHeadlessDriver';
import { AgySession } from '../session';
import { MessageQueue2 } from '@/utils/MessageQueue2';
import { resetPeerToolsForTests } from '@/modules/common/peerTools';
import type { AgyMode, PermissionMode } from '../types';

// A fake agy binary: emits a fixed NDJSON stream for one turn, then exits 0.
Expand Down Expand Up @@ -1409,3 +1410,43 @@ describe('AgyHeadlessDriver', () => {
expect(String(errorEvents[0][0].message)).toContain('exited with code 2');
}, 15000);
});

describe('buildAgySpawnEnv peer-tools pinning (#2056)', () => {
const original = process.env.HAPI_DISABLE_PEER_TOOLS;
const originalSshClient = process.env.SSH_CLIENT;

afterEach(() => {
if (original === undefined) {
delete process.env.HAPI_DISABLE_PEER_TOOLS;
} else {
process.env.HAPI_DISABLE_PEER_TOOLS = original;
}
if (originalSshClient === undefined) {
delete process.env.SSH_CLIENT;
} else {
process.env.SSH_CLIENT = originalSshClient;
}
resetPeerToolsForTests();
});

it('pins the session snapshot and keeps the agy hardening vars', () => {
delete process.env.HAPI_DISABLE_PEER_TOOLS;
process.env.SSH_CLIENT = '1.2.3.4 55555 22';
const env = buildAgySpawnEnv(true);
expect(env.HAPI_DISABLE_PEER_TOOLS).toBe('1');
expect(env.GEMINI_FORCE_FILE_STORAGE).toBe('true');
expect(env.SSH_CLIENT).toBeUndefined();
});

it('pins an explicit enabled snapshot', () => {
delete process.env.HAPI_DISABLE_PEER_TOOLS;
expect(buildAgySpawnEnv(false).HAPI_DISABLE_PEER_TOOLS).toBe('0');
});

it('keeps a machine-level deploy override authoritative and does not mutate process.env', () => {
process.env.HAPI_DISABLE_PEER_TOOLS = '1';
const env = buildAgySpawnEnv(false);
expect(env.HAPI_DISABLE_PEER_TOOLS).toBe('1');
expect(process.env.GEMINI_FORCE_FILE_STORAGE).toBeUndefined();
});
});
11 changes: 8 additions & 3 deletions cli/src/agy/headless/agyHeadlessDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { resolveAgyTurnModels } from '../utils/agyConversationModel';
import { killProcessByChildProcess } from '@/utils/process';
import { AGY_MODEL_LABELS } from '@hapi/protocol';
import { getAgentLaunchCommand } from '@/agent/agentLaunchCommand';
import { isPeerToolsDisabled, withPeerToolsPreference } from '@/modules/common/peerTools';

const AGY_PRINT_TIMEOUT = '30m';

Expand Down Expand Up @@ -76,13 +77,17 @@ export function buildAgyHeadlessArgs(opts: {
* the forwarded SSH agent. Only the session markers are removed (so agy does not
* classify the run as an SSH session and switch auth paths); SSH_AUTH_SOCK and
* SSH_AGENT_PID are kept for tool subprocesses.
*
* The session's peer-tools preference is pinned on top (#2056) so an agy agent
* shelling `hapi ping-peer` resolves it in the fresh child process; a non-empty
* machine-level HAPI_DISABLE_PEER_TOOLS stays authoritative (no overlay).
*/
function buildAgySpawnEnv(): NodeJS.ProcessEnv {
export function buildAgySpawnEnv(peerToolsDisabled?: boolean): NodeJS.ProcessEnv {
const env: NodeJS.ProcessEnv = { ...process.env, GEMINI_FORCE_FILE_STORAGE: 'true' }
for (const key of ['SSH_CLIENT', 'SSH_CONNECTION', 'SSH_TTY']) {
delete env[key]
}
return env
return withPeerToolsPreference(peerToolsDisabled ?? isPeerToolsDisabled(), env)
}

export type AgyHeadlessDriverOptions = {
Expand Down Expand Up @@ -121,7 +126,7 @@ export class AgyHeadlessDriver extends RemoteLauncherBase {
const child = spawn(getAgentLaunchCommand('agy'), args, {
cwd,
stdio: ['pipe', 'pipe', 'pipe'],
env: buildAgySpawnEnv(),
env: buildAgySpawnEnv(this.session.client.peerToolsDisabled),
}) as ChildProcessWithoutNullStreams;
return child;
});
Expand Down
36 changes: 32 additions & 4 deletions cli/src/api/api.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import axios from 'axios'
import type { AgentState, ClearOpencodeSessionCallbackRequest, ClearOpencodeSessionResponse, CreateMachineResponse, CreateSessionResponse, RunnerState, Machine, MachineMetadata, Metadata, Session } from '@/api/types'
import { applyHubSessionSummaryContract } from '@/modules/common/sessionSummaryInstruction'
import { applyHubPeerToolsDisabled } from '@/modules/common/peerTools'
import type { LocalResumeTarget, ResumableSession } from '@hapi/protocol'
import {
AgentStateSchema,
Expand All @@ -22,6 +23,15 @@ import { ApiMachineClient } from './apiMachine'
import { ApiSessionClient, type ApiSessionClientOptions } from './apiSession'
import { buildHubRequestHeaders } from './hubExtraHeaders'

/**
* Session row plus the hub-level peer-tools preference captured from this
* session's own create/fetch response. The bootstrap-time snapshot is
* authoritative for everything derived from this session (tool registration,
* prompts, wrapped-agent child env); absent when an older hub does not send
* it.
*/
export type SessionBootstrapInfo = Session & { peerToolsDisabled?: boolean }

export class ApiClient {
static async create(): Promise<ApiClient> {
return new ApiClient(getAuthToken())
Expand Down Expand Up @@ -53,7 +63,7 @@ export class ApiClient {
}
timeoutMs?: number
signal?: AbortSignal
}): Promise<Session> {
}): Promise<SessionBootstrapInfo> {
const response = await axios.post<CreateSessionResponse>(
`${configuration.apiUrl}/cli/sessions`,
{
Expand Down Expand Up @@ -91,6 +101,9 @@ export class ApiClient {
if (typeof parsed.data.sessionSummaryContract === 'boolean') {
applyHubSessionSummaryContract(parsed.data.sessionSummaryContract)
}
if (typeof parsed.data.disablePeerTools === 'boolean') {
applyHubPeerToolsDisabled(parsed.data.disablePeerTools)
}

const raw = parsed.data.session

Expand Down Expand Up @@ -126,11 +139,17 @@ export class ApiClient {
effort: raw.effort,
serviceTier: raw.serviceTier,
permissionMode: raw.permissionMode,
collaborationMode: raw.collaborationMode
collaborationMode: raw.collaborationMode,
// Create-time peer-tools snapshot rides the row so downstream
// session state (ApiSessionClient) captures its own preference
// instead of the process-wide module value.
...(typeof parsed.data.disablePeerTools === 'boolean'
? { peerToolsDisabled: parsed.data.disablePeerTools }
: {})
}
}

async getSession(sessionId: string): Promise<Session> {
async getSession(sessionId: string): Promise<SessionBootstrapInfo> {
const response = await axios.get(
`${configuration.apiUrl}/cli/sessions/${encodeURIComponent(sessionId)}`,
{
Expand All @@ -147,6 +166,9 @@ export class ApiClient {
if (typeof parsed.data.sessionSummaryContract === 'boolean') {
applyHubSessionSummaryContract(parsed.data.sessionSummaryContract)
}
if (typeof parsed.data.disablePeerTools === 'boolean') {
applyHubPeerToolsDisabled(parsed.data.disablePeerTools)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Minor] Capture the preference on session fetches too

getSession updates only the module global and omits peerToolsDisabled from its returned row. Consequently, bootstrapExistingSession creates an ApiSessionClient without a snapshot. Cold-resume shared Codex root A with tools enabled, enable the hub toggle, and create root B before A initializes its HTTP MCP connection: B changes the global, and A's first forwarded tool call creates an MCP server without peer tools. A's prompt and advertised bridge still expose those tools, violating new-session-only behavior.

Suggested fix

Return SessionBootstrapInfo from getSession and attach the preference as on the create path. Add coverage for a resumed shared root whose MCP connection initializes after another root bootstraps with the opposite preference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 912c873 — getSession now returns SessionBootstrapInfo and spreads disablePeerTools when the hub sends it, so bootstrapExistingSession (cold resume) captures its own snapshot through api.sessionSyncClient(sessionInfo) instead of racing the process-wide module value. Covered by the capture-resolution tests in apiSession.test.ts.

}

const raw = parsed.data.session
const metadata = (() => {
Expand Down Expand Up @@ -180,7 +202,13 @@ export class ApiClient {
effort: raw.effort,
serviceTier: raw.serviceTier,
permissionMode: raw.permissionMode,
collaborationMode: raw.collaborationMode
collaborationMode: raw.collaborationMode,
// Bootstrap-time peer-tools snapshot rides the row so existing-
// session bootstraps (cold resume) capture their own preference
// instead of racing the process-wide module value.
...(typeof parsed.data.disablePeerTools === 'boolean'
? { peerToolsDisabled: parsed.data.disablePeerTools }
: {})
}
}

Expand Down
83 changes: 82 additions & 1 deletion cli/src/api/apiSession.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { describe, expect, it, vi } from 'vitest'
import { afterEach, describe, expect, it, vi } from 'vitest'
import type { Session } from './types'
import type { SessionBootstrapInfo } from './api'
import { resetPeerToolsForTests } from '@/modules/common/peerTools'

const socketHarness = vi.hoisted(() => ({
sockets: [] as Array<{
Expand Down Expand Up @@ -170,6 +172,85 @@ function triggerIncomingUserMessage(
})
}

describe('ApiSessionClient peer-tools snapshot (#2056)', () => {
const originalEnv = process.env.HAPI_DISABLE_PEER_TOOLS

afterEach(() => {
if (originalEnv === undefined) {
delete process.env.HAPI_DISABLE_PEER_TOOLS
} else {
process.env.HAPI_DISABLE_PEER_TOOLS = originalEnv
}
resetPeerToolsForTests()
})

it('captures the create-time preference from the bootstrap row', () => {
socketHarness.sockets.length = 0
const client = new ApiSessionClient('token', { ...createSession(), peerToolsDisabled: true } as SessionBootstrapInfo)
expect(client.peerToolsDisabled).toBe(true)
client.close()
})

it('stays undefined when an older hub does not send the preference', () => {
socketHarness.sockets.length = 0
const client = new ApiSessionClient('token', createSession())
expect(client.peerToolsDisabled).toBeUndefined()
client.close()
})

it('adopts the snapshot when a pending session lazy-materializes', async () => {
socketHarness.sockets.length = 0
const materialize = vi.fn(async () => createSession({ namespace: 'default' }))
const client = new ApiSessionClient('token', createSession(), { materialize })
client.sendUserMessage('hello')
expect(await client.materialize()).toBe(true)
expect(client.peerToolsDisabled).toBeUndefined()
client.close()

socketHarness.sockets.length = 0
const materializeDisabled = vi.fn(async () => ({ ...createSession({ namespace: 'default' }), peerToolsDisabled: true } as SessionBootstrapInfo))
const adopted = new ApiSessionClient('token', createSession(), { materialize: materializeDisabled })
adopted.sendUserMessage('hello')
expect(await adopted.materialize()).toBe(true)
expect(adopted.peerToolsDisabled).toBe(true)
adopted.close()
})

it('resolves the snapshot through the machine env at capture time (both directions)', () => {
socketHarness.sockets.length = 0
delete process.env.HAPI_DISABLE_PEER_TOOLS
const faithful = new ApiSessionClient('token', { ...createSession(), peerToolsDisabled: true } as SessionBootstrapInfo)
expect(faithful.peerToolsDisabled).toBe(true)
faithful.close()

socketHarness.sockets.length = 0
// A non-empty deploy override must beat the hub value in BOTH
// directions at capture, so the stored snapshot never flips later
// with the process-wide module value (#2056 round 2).
process.env.HAPI_DISABLE_PEER_TOOLS = '1'
const machineWinsDisable = new ApiSessionClient('token', { ...createSession(), peerToolsDisabled: false } as SessionBootstrapInfo)
expect(machineWinsDisable.peerToolsDisabled).toBe(true)
machineWinsDisable.close()

socketHarness.sockets.length = 0
process.env.HAPI_DISABLE_PEER_TOOLS = '0'
const machineWinsEnable = new ApiSessionClient('token', { ...createSession(), peerToolsDisabled: true } as SessionBootstrapInfo)
expect(machineWinsEnable.peerToolsDisabled).toBe(false)
machineWinsEnable.close()
})

it('resolves the adopted snapshot through the machine env too (lazy materialization)', async () => {
socketHarness.sockets.length = 0
process.env.HAPI_DISABLE_PEER_TOOLS = '0'
const materializeDisabled = vi.fn(async () => ({ ...createSession({ namespace: 'default' }), peerToolsDisabled: true } as SessionBootstrapInfo))
const adopted = new ApiSessionClient('token', createSession(), { materialize: materializeDisabled })
adopted.sendUserMessage('hello')
expect(await adopted.materialize()).toBe(true)
expect(adopted.peerToolsDisabled).toBe(false)
adopted.close()
})
})

describe('ApiSessionClient lazy materialization', () => {
it('does not connect or materialize without a real user message', async () => {
socketHarness.sockets.length = 0
Expand Down
19 changes: 19 additions & 0 deletions cli/src/api/apiSession.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import type { ZodType } from 'zod'
import { logger } from '@/ui/logger'
import { backoff } from '@/utils/time'
import { apiValidationError } from '@/utils/errorUtils'
import type { SessionBootstrapInfo } from './api'
import { resolveSessionPeerToolsDisabled } from '@/modules/common/peerTools'
import { AsyncLock } from '@/utils/lock'
import type { RawJSONLines } from '@/claude/types'
import { configuration } from '@/configuration'
Expand Down Expand Up @@ -243,6 +245,17 @@ export class ApiSessionClient extends EventEmitter {
* the handler must read this synchronously at registration.
*/
hubArchived = false
/**
* Peer-tools kill switch captured from THIS session's create/fetch
* response and resolved through the machine-level deploy override at
* capture time (a non-empty HAPI_DISABLE_PEER_TOOLS beats the hub value in
* both directions; undefined when an older hub does not send it, or until
* a lazy pending session materializes). Authoritative for tool
* registration, prompts and wrapped-agent child env derived from this
* session; the module-wide preference only serves as fallback (#2056
* review: keep the resolved preference scoped per session/root).
*/
peerToolsDisabled?: boolean
private readonly token: string
readonly sessionId: string
private metadata: Metadata | null
Expand Down Expand Up @@ -299,6 +312,8 @@ export class ApiSessionClient extends EventEmitter {
this.metadataVersion = session.metadataVersion
this.agentState = session.agentState
this.agentStateVersion = session.agentStateVersion
const bootstrapSnapshot = (session as SessionBootstrapInfo).peerToolsDisabled
this.peerToolsDisabled = bootstrapSnapshot === undefined ? undefined : resolveSessionPeerToolsDisabled(bootstrapSnapshot)
this.materializer = options.materialize
this.onMaterialized = options.onMaterialized
this.state = this.materializer ? 'pending' : 'active'
Expand Down Expand Up @@ -606,6 +621,10 @@ export class ApiSessionClient extends EventEmitter {
this.metadataVersion = materialized.metadataVersion
this.agentState = materialized.agentState
this.agentStateVersion = materialized.agentStateVersion
const materializedSnapshot = (materialized as SessionBootstrapInfo).peerToolsDisabled
this.peerToolsDisabled = materializedSnapshot === undefined
? this.peerToolsDisabled
: resolveSessionPeerToolsDisabled(materializedSnapshot)
this.state = 'active'

if (shouldSyncMetadata && latestMetadata) {
Expand Down
46 changes: 45 additions & 1 deletion cli/src/claude/claudeLocal.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

const harness = vi.hoisted(() => ({
spawn: vi.fn(async (_opts: unknown) => {})
Expand Down Expand Up @@ -49,6 +49,7 @@ vi.mock('./sdk/utils', () => ({
}))

import { claudeLocal } from './claudeLocal'
import { resetPeerToolsForTests } from '@/modules/common/peerTools'

function getSpawnArgs(): string[] {
const call = harness.spawn.mock.calls[0]
Expand Down Expand Up @@ -113,3 +114,46 @@ describe('claudeLocal model arguments', () => {
expect(args).toContain('--verbose')
})
})

describe('claudeLocal peer-tools child env (#2056)', () => {
const original = process.env.HAPI_DISABLE_PEER_TOOLS

beforeEach(() => {
harness.spawn.mockClear()
})

afterEach(() => {
if (original === undefined) delete process.env.HAPI_DISABLE_PEER_TOOLS
else process.env.HAPI_DISABLE_PEER_TOOLS = original
resetPeerToolsForTests()
})

async function spawnEnv(peerToolsDisabled?: boolean): Promise<Record<string, string | undefined>> {
await claudeLocal({
abort: new AbortController().signal,
sessionId: null,
path: '/workspace',
hookSettingsPath: '/tmp/hooks.json',
...(peerToolsDisabled === undefined ? {} : { peerToolsDisabled })
})
const call = harness.spawn.mock.calls[0]
expect(call).toBeDefined()
return (call[0] as { env: Record<string, string | undefined> }).env
}

it('pins a disabled session snapshot as an explicit env value', async () => {
delete process.env.HAPI_DISABLE_PEER_TOOLS
expect((await spawnEnv(true)).HAPI_DISABLE_PEER_TOOLS).toBe('1')
})

it('pins an enabled session explicitly so a later hub flip cannot rewrite the child', async () => {
delete process.env.HAPI_DISABLE_PEER_TOOLS
expect((await spawnEnv(false)).HAPI_DISABLE_PEER_TOOLS).toBe('0')
})

it('keeps a machine-level override authoritative (no session overlay)', async () => {
process.env.HAPI_DISABLE_PEER_TOOLS = '1'
const env = await spawnEnv(false)
expect(env.HAPI_DISABLE_PEER_TOOLS).toBe('1')
})
})
Loading
Loading