fix(workspace-trust): harden Claude preflight for live teams
This commit is contained in:
parent
3d0ee71d09
commit
3908b5eb25
4 changed files with 101 additions and 11 deletions
|
|
@ -11,6 +11,12 @@ import type {
|
|||
} from './ports';
|
||||
|
||||
const WORKSPACE_TRUST_RAW_TAIL_LIMIT = 4096;
|
||||
const CLAUDE_WORKSPACE_TRUST_PREFLIGHT_TIMEOUT_MS = 60_000;
|
||||
const CLAUDE_WORKSPACE_TRUST_CONFIRM_TIMEOUT_MS = 5_000;
|
||||
|
||||
function sleep(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
export interface ClaudePtyWorkspaceTrustStrategyInput {
|
||||
claudePath: string;
|
||||
|
|
@ -45,6 +51,27 @@ function buildRawTail(snapshot: TerminalSnapshot | undefined): string | undefine
|
|||
return normalized.slice(-WORKSPACE_TRUST_RAW_TAIL_LIMIT);
|
||||
}
|
||||
|
||||
async function waitForTrustedState(input: {
|
||||
stateProbe: ProviderStateProbe;
|
||||
workspace: WorkspaceTrustWorkspace;
|
||||
isCancelled(): boolean;
|
||||
timeoutMs: number;
|
||||
pollIntervalMs: number;
|
||||
}): Promise<Awaited<ReturnType<ProviderStateProbe['readTrustState']>>> {
|
||||
const pollIntervalMs = Math.max(1, input.pollIntervalMs);
|
||||
const deadline = Date.now() + input.timeoutMs;
|
||||
let last = await input.stateProbe.readTrustState(input.workspace);
|
||||
while (last.status !== 'trusted' && !input.isCancelled()) {
|
||||
const remainingMs = deadline - Date.now();
|
||||
if (remainingMs <= 0) {
|
||||
break;
|
||||
}
|
||||
await sleep(Math.min(pollIntervalMs, remainingMs));
|
||||
last = await input.stateProbe.readTrustState(input.workspace);
|
||||
}
|
||||
return last;
|
||||
}
|
||||
|
||||
function worseStatus(
|
||||
current: WorkspaceTrustDiagnosticStrategyResult['status'],
|
||||
next: WorkspaceTrustDiagnosticStrategyResult['status']
|
||||
|
|
@ -152,13 +179,22 @@ export class ClaudePtyWorkspaceTrustStrategy {
|
|||
session: spawnResult.session,
|
||||
detect: detectClaudeStartupState,
|
||||
isCancelled: input.isCancelled,
|
||||
timeoutMs: input.timeoutMs,
|
||||
timeoutMs: input.timeoutMs ?? CLAUDE_WORKSPACE_TRUST_PREFLIGHT_TIMEOUT_MS,
|
||||
pollIntervalMs: input.pollIntervalMs,
|
||||
afterDialogAction: async ({ ruleId }) => {
|
||||
if (ruleId !== 'claude.workspace_trust') {
|
||||
return { action: 'continue' };
|
||||
}
|
||||
const after = await stateProbe.readTrustState(workspace);
|
||||
const after = await waitForTrustedState({
|
||||
stateProbe,
|
||||
workspace,
|
||||
isCancelled: input.isCancelled,
|
||||
timeoutMs: Math.min(
|
||||
CLAUDE_WORKSPACE_TRUST_CONFIRM_TIMEOUT_MS,
|
||||
input.timeoutMs ?? CLAUDE_WORKSPACE_TRUST_CONFIRM_TIMEOUT_MS
|
||||
),
|
||||
pollIntervalMs: input.pollIntervalMs ?? 100,
|
||||
});
|
||||
if (after.status === 'trusted') {
|
||||
evidence.push(...after.evidence);
|
||||
return { action: 'stop', reason: 'workspace_trust_persisted' };
|
||||
|
|
@ -218,7 +254,7 @@ export class ClaudePtyWorkspaceTrustStrategy {
|
|||
workspaceIds,
|
||||
matchedRuleIds: [...new Set(matchedRuleIds)],
|
||||
actions,
|
||||
evidence,
|
||||
evidence: [...new Set(evidence)],
|
||||
elapsedMs: Date.now() - startedAt,
|
||||
errorCode,
|
||||
errorMessage,
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import { createRequire } from 'node:module';
|
||||
|
||||
import { createLogger } from '@shared/utils/logger';
|
||||
|
||||
import type {
|
||||
|
|
@ -13,6 +15,7 @@ import type * as NodePty from 'node-pty';
|
|||
|
||||
const logger = createLogger('WorkspaceTrustNodePtyProcessAdapter');
|
||||
const MAX_TRANSCRIPT_CHARS = 64 * 1024;
|
||||
const requireNativeAddon = createRequire(import.meta.url);
|
||||
|
||||
type NodePtyModule = typeof NodePty;
|
||||
|
||||
|
|
@ -23,8 +26,7 @@ function loadNodePty(): NodePtyModule | null {
|
|||
return nodePty;
|
||||
}
|
||||
try {
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports -- node-pty is optional native addon
|
||||
nodePty = require('node-pty') as NodePtyModule;
|
||||
nodePty = requireNativeAddon('node-pty') as NodePtyModule;
|
||||
} catch (error) {
|
||||
logger.warn(`node-pty unavailable for workspace trust preflight: ${String(error)}`);
|
||||
nodePty = null;
|
||||
|
|
|
|||
|
|
@ -1,7 +1,6 @@
|
|||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { ClaudePtyWorkspaceTrustStrategy } from '@features/workspace-trust/core/application';
|
||||
import { buildWorkspaceTrustPathCandidates } from '@features/workspace-trust/core/domain';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import type {
|
||||
ProviderStateProbe,
|
||||
|
|
@ -83,6 +82,10 @@ function workspace(cwd = '/tmp/project') {
|
|||
}
|
||||
|
||||
describe('ClaudePtyWorkspaceTrustStrategy', () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('skips PTY when the state probe already reports trusted', async () => {
|
||||
const pty = new FakePtyProcess();
|
||||
const result = await new ClaudePtyWorkspaceTrustStrategy().execute({
|
||||
|
|
@ -172,15 +175,17 @@ describe('ClaudePtyWorkspaceTrustStrategy', () => {
|
|||
it('accepts the trust dialog, verifies persisted trust, kills PTY, and cleans temp MCP config', async () => {
|
||||
const pty = new FakePtyProcess();
|
||||
const tempStore = new FakeTempStore();
|
||||
const stateProbe = new FakeStateProbe([
|
||||
{ status: 'untrusted' },
|
||||
{ status: 'untrusted' },
|
||||
{ status: 'trusted', evidence: ['trusted project key: /tmp/project'] },
|
||||
]);
|
||||
const result = await new ClaudePtyWorkspaceTrustStrategy().execute({
|
||||
claudePath: '/usr/local/bin/claude',
|
||||
workspaces: [workspace()],
|
||||
env: { HOME: '/Users/tester', PATH: '/usr/local/bin', OPTIONAL_EMPTY: undefined },
|
||||
ptyProcess: pty,
|
||||
stateProbe: new FakeStateProbe([
|
||||
{ status: 'untrusted' },
|
||||
{ status: 'trusted', evidence: ['trusted project key: /tmp/project'] },
|
||||
]),
|
||||
stateProbe,
|
||||
tempEmptyMcpConfigStore: tempStore,
|
||||
isCancelled: () => false,
|
||||
timeoutMs: 100,
|
||||
|
|
@ -190,6 +195,7 @@ describe('ClaudePtyWorkspaceTrustStrategy', () => {
|
|||
expect(result.status).toBe('ok');
|
||||
expect(result.matchedRuleIds).toEqual(['claude.workspace_trust']);
|
||||
expect(result.actions).toEqual(['claude.workspace_trust:enter']);
|
||||
expect(result.evidence).toEqual(['trusted project key: /tmp/project']);
|
||||
expect(pty.spawnInputs[0]).toMatchObject({
|
||||
command: '/usr/local/bin/claude',
|
||||
cwd: '/tmp/project',
|
||||
|
|
@ -203,6 +209,33 @@ describe('ClaudePtyWorkspaceTrustStrategy', () => {
|
|||
expect(pty.session?.actions.map((action) => action.id)).toEqual(['enter']);
|
||||
expect(pty.session?.killed).toBe(true);
|
||||
expect(tempStore.cleaned).toBe(true);
|
||||
expect(stateProbe.calls).toBe(4);
|
||||
});
|
||||
|
||||
it('keeps the default Claude preflight alive long enough for slow startup trust prompts', async () => {
|
||||
const pty = new FakePtyProcess();
|
||||
const session = new FakeSession(['Starting Claude...', 'Quick safety check\nYes, I trust this folder']);
|
||||
pty.spawnResult = { ok: true, session };
|
||||
const nowValues = [0, 0, 0, 0, 46_000, 46_000, 46_000];
|
||||
vi.spyOn(Date, 'now').mockImplementation(() => nowValues.shift() ?? 46_000);
|
||||
|
||||
const result = await new ClaudePtyWorkspaceTrustStrategy().execute({
|
||||
claudePath: '/usr/local/bin/claude',
|
||||
workspaces: [workspace()],
|
||||
env: { HOME: '/Users/tester' },
|
||||
ptyProcess: pty,
|
||||
stateProbe: new FakeStateProbe([
|
||||
{ status: 'untrusted' },
|
||||
{ status: 'trusted', evidence: ['trusted project key: /tmp/project'] },
|
||||
]),
|
||||
tempEmptyMcpConfigStore: new FakeTempStore(),
|
||||
isCancelled: () => false,
|
||||
pollIntervalMs: 1,
|
||||
});
|
||||
|
||||
expect(result.status).toBe('ok');
|
||||
expect(result.matchedRuleIds).toEqual(['claude.workspace_trust']);
|
||||
expect(session.actions.map((action) => action.id)).toEqual(['enter']);
|
||||
});
|
||||
|
||||
it('soft-fails when node-pty is unavailable instead of throwing', async () => {
|
||||
|
|
|
|||
|
|
@ -5,10 +5,14 @@ import * as path from 'node:path';
|
|||
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { createWorkspaceTrustCoordinator } from '../../../../src/features/workspace-trust/main';
|
||||
import { TeamDataService } from '../../../../src/main/services/team/TeamDataService';
|
||||
import { TeamProvisioningService } from '../../../../src/main/services/team/TeamProvisioningService';
|
||||
import { TeamTaskReader } from '../../../../src/main/services/team/TeamTaskReader';
|
||||
import {
|
||||
getAutoDetectedClaudeBasePath,
|
||||
getClaudeBasePath,
|
||||
getHomeDir,
|
||||
getTasksBasePath,
|
||||
getTeamsBasePath,
|
||||
setClaudeBasePathOverride,
|
||||
|
|
@ -236,6 +240,7 @@ async function runProviderStressScenario(
|
|||
throw error;
|
||||
}
|
||||
const svc = harness?.svc ?? new TeamProvisioningService();
|
||||
configureWorkspaceTrustCoordinator(svc);
|
||||
const active: ActiveScenario = { scenario, teamName, svc, harness, codexCleanup, failed: false };
|
||||
activeScenarios.push(active);
|
||||
|
||||
|
|
@ -296,6 +301,20 @@ async function runProviderStressScenario(
|
|||
}
|
||||
}
|
||||
|
||||
function configureWorkspaceTrustCoordinator(svc: TeamProvisioningService): void {
|
||||
svc.setWorkspaceTrustCoordinator(
|
||||
createWorkspaceTrustCoordinator({
|
||||
claudeConfigDir: () => getClaudeBasePath(),
|
||||
globalConfigFilePath: () => {
|
||||
const claudeBasePath = getClaudeBasePath();
|
||||
return claudeBasePath !== getAutoDetectedClaudeBasePath()
|
||||
? path.join(claudeBasePath, '.claude.json')
|
||||
: path.join(getHomeDir(), '.claude.json');
|
||||
},
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
async function runRestartStressChecks(
|
||||
active: ActiveScenario,
|
||||
expectedMembers: string[],
|
||||
|
|
|
|||
Loading…
Reference in a new issue