Skip to content

Commit f7b0567

Browse files
vilmireclaude
andcommitted
fix(mesh): restore coordinator mark + auto-approve on daemon restart
restoreHostedSessions recreated re-attached CLI runtimes with a bare {} settings object, diverging from a fresh launch which seeds settings with { ...providerLoader.getSettings(type), ...settingsOverride }. On a daemon restart the empty object silently dropped two launch settings from the restored instance: - autoApprove (a provider/machine setting from getSettings) — a restored coordinator self-session lost auto-approve and re-prompted for manual approval on every mesh tool call. - meshCoordinatorFor (the coordinator launch's settingsOverride) — the restored session was no longer recognized as this daemon's live CLI coordinator by findLiveCoordinators (so pending mesh events stopped draining into its PTY) and lost the coordinator badge signal that the status builder derives from settings. Re-establish both on restore: seed from providerLoader.getSettings(type) (matching a fresh launch, so autoApprove and other provider defaults come back for every restored session) and merge meshCoordinatorFor back from the persisted MeshCoordinatorRegistry (loaded on boot, keyed by the stable runtimeId) when the restored runtime was a registered coordinator. Both restores are provider-agnostic. Add a regression test asserting a registered-coordinator restore carries autoApprove + meshCoordinatorFor while a plain session restore carries autoApprove only (no invented coordinator mark). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
1 parent d465e28 commit f7b0567

2 files changed

Lines changed: 172 additions & 2 deletions

File tree

packages/daemon-core/src/commands/cli-manager.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { loadConfig } from '../config/config.js';
1818
import { loadState, saveState } from '../config/state-store.js';
1919
import { getWorkspaceState, resolveLaunchDirectory } from '../config/workspaces.js';
2020
import { appendRecentActivity } from '../config/recent-activity.js';
21-
import { unregisterMeshCoordinator } from '../mesh/coordinator-registry.js';
21+
import { unregisterMeshCoordinator, getCoordinatorForSession } from '../mesh/coordinator-registry.js';
2222
import { upsertSavedProviderSession } from '../config/saved-sessions.js';
2323
import { buildLegacyModelModeSummaryMetadata, normalizeProviderSummaryMetadata } from '../providers/summary-metadata.js';
2424
import { CliProviderInstance } from '../providers/cli-provider-instance.js';
@@ -1042,6 +1042,24 @@ export class DaemonCliManager {
10421042
);
10431043
continue;
10441044
}
1045+
// Re-establish the launch-time settings a fresh launch applies. startSession
1046+
// seeds every new instance with { ...providerLoader.getSettings(type), ...override };
1047+
// passing a bare {} here on restart silently dropped TWO launch settings, so a
1048+
// restored session diverged from a freshly-launched one:
1049+
// - autoApprove (a provider/machine setting from getSettings) → a restored
1050+
// coordinator self-session lost auto-approve and re-prompted on every tool call.
1051+
// - meshCoordinatorFor (the coordinator launch's settingsOverride) → the restored
1052+
// session was no longer recognized as this daemon's live CLI coordinator by
1053+
// findLiveCoordinators (so pending mesh events stopped draining into its PTY) nor
1054+
// surfaced with the coordinator badge via settings. The persisted coordinator
1055+
// registry (loaded on boot) is the source of truth to rebuild that mark.
1056+
// Both restores are provider-agnostic — getSettings is keyed by provider type and the
1057+
// registry mark is type-independent.
1058+
const restoredSettings: Record<string, any> = { ...this.providerLoader.getSettings(normalizedType) };
1059+
const coordinatorEntry = getCoordinatorForSession(record.runtimeId);
1060+
if (coordinatorEntry?.meshId) {
1061+
restoredSettings.meshCoordinatorFor = coordinatorEntry.meshId;
1062+
}
10451063
try {
10461064
await this.registerCliInstance(
10471065
record.runtimeId,
@@ -1050,7 +1068,7 @@ export class DaemonCliManager {
10501068
record.workspace,
10511069
record.cliArgs,
10521070
resolvedProvider,
1053-
{},
1071+
restoredSettings,
10541072
true,
10551073
{
10561074
providerSessionId: sessionBinding.providerSessionId,
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
import { chmodSync, mkdtempSync, rmSync, mkdirSync, writeFileSync } from 'fs';
2+
import { join } from 'path';
3+
import { tmpdir } from 'os';
4+
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
5+
import { DaemonCliManager } from '../../src/commands/cli-manager.js';
6+
import { ProviderLoader } from '../../src/providers/provider-loader.js';
7+
import { registerMeshCoordinator, unregisterMeshCoordinator } from '../../src/mesh/coordinator-registry.js';
8+
9+
// Regression: a daemon restart re-attaches hosted CLI runtimes via
10+
// restoreHostedSessions. That path used to recreate instances with a bare {}
11+
// settings object, diverging from a fresh launch (which seeds settings with
12+
// providerLoader.getSettings(type) + the launch settingsOverride). The drop
13+
// silently lost autoApprove (a provider/machine setting) AND meshCoordinatorFor
14+
// (the coordinator launch override), so a restarted coordinator self-session
15+
// lost auto-approve and stopped being recognized as a live coordinator. This
16+
// asserts both launch settings are re-established on restore — provider-agnostic.
17+
18+
function writeProvider(root: string, category: string, type: string, data: Record<string, unknown>) {
19+
const dir = join(root, category, type);
20+
mkdirSync(dir, { recursive: true });
21+
writeFileSync(join(dir, 'provider.json'), JSON.stringify(data), 'utf-8');
22+
}
23+
24+
class TestProviderLoader extends ProviderLoader {
25+
constructor(
26+
userDir: string,
27+
private readonly testConfig: {
28+
machineProviders?: Record<string, { enabled?: boolean; executable?: string; args?: string[] }>;
29+
providerSettings?: Record<string, Record<string, unknown>>;
30+
},
31+
) {
32+
super({ userDir, disableUpstream: true });
33+
}
34+
35+
protected override readConfig(): any | null {
36+
return this.testConfig;
37+
}
38+
39+
protected override writeConfig(config: any): void {
40+
Object.assign(this.testConfig, config);
41+
}
42+
}
43+
44+
function createManager(loader: ProviderLoader, overrides: Partial<{
45+
getInstanceManager: () => any;
46+
getSessionRegistry: () => any;
47+
}> = {}) {
48+
return new DaemonCliManager({
49+
getServerConn: () => null,
50+
getP2p: () => null,
51+
onStatusChange: vi.fn(),
52+
removeAgentTracking: vi.fn(),
53+
getInstanceManager: overrides.getInstanceManager || (() => null),
54+
getSessionRegistry: overrides.getSessionRegistry || (() => null),
55+
}, loader);
56+
}
57+
58+
describe('DaemonCliManager.restoreHostedSessions re-establishes launch settings', () => {
59+
let providerRoot = '';
60+
let workingDir = '';
61+
let configDir = '';
62+
let prevConfigDir: string | undefined;
63+
let testConfig: { machineProviders: Record<string, { enabled?: boolean; executable?: string; args?: string[] }>; providerSettings: Record<string, Record<string, unknown>> };
64+
65+
beforeEach(() => {
66+
providerRoot = mkdtempSync(join(tmpdir(), 'adhdev-restore-providers-'));
67+
workingDir = mkdtempSync(join(tmpdir(), 'adhdev-restore-workspace-'));
68+
configDir = mkdtempSync(join(tmpdir(), 'adhdev-restore-config-'));
69+
// Isolate the persisted coordinator registry writes (mesh-coordinators.json)
70+
// to a temp config dir so the test never touches the real ~/.adhdev.
71+
prevConfigDir = process.env.ADHDEV_CONFIG_DIR;
72+
process.env.ADHDEV_CONFIG_DIR = configDir;
73+
testConfig = { machineProviders: {}, providerSettings: {} };
74+
});
75+
76+
afterEach(() => {
77+
if (prevConfigDir === undefined) delete process.env.ADHDEV_CONFIG_DIR;
78+
else process.env.ADHDEV_CONFIG_DIR = prevConfigDir;
79+
if (providerRoot) rmSync(providerRoot, { recursive: true, force: true });
80+
if (workingDir) rmSync(workingDir, { recursive: true, force: true });
81+
if (configDir) rmSync(configDir, { recursive: true, force: true });
82+
});
83+
84+
function setupLoader() {
85+
const executable = join(providerRoot, 'bin', 'sample-cli');
86+
mkdirSync(join(providerRoot, 'bin'), { recursive: true });
87+
writeFileSync(executable, '#!/bin/sh\nexit 0\n', 'utf-8');
88+
chmodSync(executable, 0o755);
89+
90+
writeProvider(providerRoot, 'cli', 'sample-cli', {
91+
type: 'sample-cli',
92+
name: 'Sample CLI',
93+
displayName: 'Sample CLI',
94+
category: 'cli',
95+
spawn: { command: 'sample-cli-definitely-missing' },
96+
patterns: ['sample'],
97+
settings: {
98+
autoApprove: { type: 'boolean', default: false, public: true },
99+
},
100+
});
101+
testConfig.machineProviders['sample-cli'] = { enabled: true, executable };
102+
testConfig.providerSettings['sample-cli'] = { autoApprove: true };
103+
const loader = new TestProviderLoader(providerRoot, testConfig);
104+
loader.loadAll();
105+
return loader;
106+
}
107+
108+
it('restores provider autoApprove and the coordinator mark for a registered coordinator session', async () => {
109+
const loader = setupLoader();
110+
const runtimeId = 'coordinator-runtime-1';
111+
const meshId = 'mesh-alpha';
112+
registerMeshCoordinator({ meshId, sessionId: runtimeId, workspace: workingDir, startedAt: 1, cliType: 'sample-cli' });
113+
114+
try {
115+
const addInstance = vi.fn();
116+
const removeInstance = vi.fn();
117+
const restored = await createManager(loader, {
118+
getInstanceManager: () => ({ addInstance, removeInstance, getInstance: () => null }),
119+
getSessionRegistry: () => ({ register: vi.fn() }),
120+
}).restoreHostedSessions([
121+
{ runtimeId, cliType: 'sample-cli', workspace: workingDir },
122+
]);
123+
124+
expect(restored).toBe(1);
125+
expect(addInstance).toHaveBeenCalledTimes(1);
126+
const context = addInstance.mock.calls[0][2] as any;
127+
expect(context.settings).toMatchObject({ autoApprove: true, meshCoordinatorFor: meshId });
128+
} finally {
129+
unregisterMeshCoordinator(runtimeId);
130+
}
131+
}, 15000);
132+
133+
it('restores provider autoApprove but does not invent a coordinator mark for a plain session', async () => {
134+
const loader = setupLoader();
135+
const runtimeId = 'plain-runtime-1';
136+
137+
const addInstance = vi.fn();
138+
const removeInstance = vi.fn();
139+
const restored = await createManager(loader, {
140+
getInstanceManager: () => ({ addInstance, removeInstance, getInstance: () => null }),
141+
getSessionRegistry: () => ({ register: vi.fn() }),
142+
}).restoreHostedSessions([
143+
{ runtimeId, cliType: 'sample-cli', workspace: workingDir },
144+
]);
145+
146+
expect(restored).toBe(1);
147+
expect(addInstance).toHaveBeenCalledTimes(1);
148+
const context = addInstance.mock.calls[0][2] as any;
149+
expect(context.settings).toMatchObject({ autoApprove: true });
150+
expect(context.settings.meshCoordinatorFor).toBeUndefined();
151+
}, 15000);
152+
});

0 commit comments

Comments
 (0)