Skip to content

Commit 139bc4a

Browse files
lilyshen0722sprint-impl
andauthored
fix(cli): resolve agent config installation on server (TASK-473) (#2660)
Read the owner-filtered Settings projection before choosing a PATCH target. Managed seats use runtimeSourcePodId; standalone seats use writablePodId. Refuse absent targets and lookup failures without falling back to the token pod. Written by Sprint Impl, a Commonly agent https://commonly.me/pods/6a692a1be833c668acdb84cf Agent-Model: codex/gpt-6.1-sol/medium Co-authored-by: sprint-impl <WyJzcHJpbnQtaW1wbCIsImRlZmF1bHQiXQ@agents.commonly.invalid>
1 parent c9c0d71 commit 139bc4a

2 files changed

Lines changed: 62 additions & 15 deletions

File tree

‎cli/__tests__/attach.test.mjs‎

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ describe('updateAgentConfiguration', () => {
146146
};
147147

148148
test('reuses the registry PATCH route for runtime controls and full env specs', async () => {
149-
const client = { patch: jest.fn(async () => ({ success: true })) };
149+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn(async () => ({ success: true })) };
150150
const environment = {
151151
version: 1,
152152
workspace: { path: './workspace' },
@@ -179,7 +179,7 @@ describe('updateAgentConfiguration', () => {
179179
});
180180

181181
test('keeps the complete provider block local while PATCHing environment fields', async () => {
182-
const client = { patch: jest.fn(async () => ({ success: true })) };
182+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn(async () => ({ success: true })) };
183183
const environment = {
184184
model: 'gpt-5.4',
185185
provider: {
@@ -199,7 +199,7 @@ describe('updateAgentConfiguration', () => {
199199
});
200200

201201
test('refuses provider configuration when the local token record is not OpenCode-bound', async () => {
202-
const client = { patch: jest.fn() };
202+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn() };
203203
await expect(updateAgentConfiguration({
204204
client,
205205
record,
@@ -215,7 +215,7 @@ describe('updateAgentConfiguration', () => {
215215
});
216216

217217
test('agent config writes the adapter binding explicitly', async () => {
218-
const client = { patch: jest.fn(async () => ({ success: true })) };
218+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn(async () => ({ success: true })) };
219219
const adapterRegistry = {
220220
listAdapterNames: () => ['opencode'],
221221
getAdapter: () => ({ detect: async () => ({ path: '/bin/opencode', version: '1.18.35' }) }),
@@ -228,17 +228,17 @@ describe('updateAgentConfiguration', () => {
228228
});
229229

230230
test('rejects an empty edit and an incomplete token record before making a request', async () => {
231-
const client = { patch: jest.fn() };
231+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn() };
232232
await expect(updateAgentConfiguration({ client, record })).rejects.toThrow(/at least one/);
233233
await expect(updateAgentConfiguration({ client, record, effort: 'turbo' }))
234234
.rejects.toThrow(/effort must be one of/);
235-
await expect(updateAgentConfiguration({ client, record: { agentName: 'juno' }, model: 'opus' }))
236-
.rejects.toThrow(/podId/);
235+
await expect(updateAgentConfiguration({ client, record: { podId: 'pod-9' }, model: 'opus' }))
236+
.rejects.toThrow(/agentName/);
237237
expect(client.patch).not.toHaveBeenCalled();
238238
});
239239

240240
test('updates an existing declared environment when only model/effort flags change', async () => {
241-
const client = { patch: jest.fn(async () => ({ success: true })) };
241+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn(async () => ({ success: true })) };
242242
const localRecord = {
243243
...record,
244244
environment: {
@@ -255,7 +255,7 @@ describe('updateAgentConfiguration', () => {
255255
});
256256

257257
test('materializes runtime edits into the declared environment when none exists', async () => {
258-
const client = { patch: jest.fn(async () => ({ success: true })) };
258+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn(async () => ({ success: true })) };
259259
const result = await updateAgentConfiguration({ client, record, model: 'new-model', effort: 'high' });
260260
expect(result.environment).toEqual({ model: 'new-model', effort: 'high' });
261261
expect(client.patch.mock.calls[0][1].config).toEqual({
@@ -265,7 +265,7 @@ describe('updateAgentConfiguration', () => {
265265
});
266266

267267
test('validates and persists a detected adapter without leaking it into ADR-008 environment', async () => {
268-
const client = { patch: jest.fn(async () => ({ success: true })) };
268+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn(async () => ({ success: true })) };
269269
const adapter = { detect: jest.fn(async () => ({ path: '/usr/local/bin/claude' })) };
270270
const result = await updateAgentConfiguration({
271271
client,
@@ -288,7 +288,7 @@ describe('updateAgentConfiguration', () => {
288288
});
289289

290290
test('rejects an unknown or unavailable adapter before PATCH', async () => {
291-
const client = { patch: jest.fn() };
291+
const client = { get: jest.fn(async () => ({ writablePodId: 'pod-9' })), patch: jest.fn() };
292292
const adapter = { detect: jest.fn(async () => null) };
293293
const adapterRegistry = {
294294
getAdapter: jest.fn((name) => (name === 'claude' ? adapter : null)),
@@ -300,6 +300,43 @@ describe('updateAgentConfiguration', () => {
300300
.rejects.toThrow(/not found on PATH/);
301301
expect(client.patch).not.toHaveBeenCalled();
302302
});
303+
test('resolves a managed seat from the server source instead of its attention pod', async () => {
304+
const client = {
305+
get: jest.fn(async () => ({ runtimeManaged: true, runtimeSourcePodId: 'source-pod', writablePodId: 'other-pod' })),
306+
patch: jest.fn(async () => ({})),
307+
};
308+
const result = await updateAgentConfiguration({ client, record, model: 'new-model' });
309+
expect(client.get).toHaveBeenCalledWith('/api/registry/agents/juno/manage-settings?instanceId=writer');
310+
expect(client.patch.mock.calls[0][0]).toBe('/api/registry/pods/source-pod/agents/juno');
311+
expect(result.podId).toBe('source-pod');
312+
});
313+
314+
test('a stale or absent local pod is not used when the server has a writable installation', async () => {
315+
const client = {
316+
get: jest.fn(async () => ({ writablePodId: 'real-pod' })),
317+
patch: jest.fn(async () => ({})),
318+
};
319+
await updateAgentConfiguration({ client, record: { ...record, podId: undefined }, model: 'new-model' });
320+
expect(client.patch.mock.calls[0][0]).toBe('/api/registry/pods/real-pod/agents/juno');
321+
});
322+
323+
test.each([
324+
{},
325+
{ writablePodId: null },
326+
{ runtimeManaged: true, runtimeSourcePodId: null, writablePodId: 'other-pod' },
327+
])('refuses a missing server target without PATCHing the token pod: %j', async (settings) => {
328+
const client = { get: jest.fn(async () => settings), patch: jest.fn() };
329+
await expect(updateAgentConfiguration({ client, record, model: 'new-model' }))
330+
.rejects.toThrow(/No writable runtime installation/);
331+
expect(client.patch).not.toHaveBeenCalled();
332+
});
333+
334+
test('a failed installation lookup never falls back to the token pod', async () => {
335+
const client = { get: jest.fn(async () => { throw new Error('Forbidden'); }), patch: jest.fn() };
336+
await expect(updateAgentConfiguration({ client, record, model: 'new-model' })).rejects.toThrow('Forbidden');
337+
expect(client.patch).not.toHaveBeenCalled();
338+
});
339+
303340
});
304341

305342
const makeClient = ({ publishOk = true, runtimeToken = null } = {}) => {

‎cli/src/commands/agent.js‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -669,8 +669,8 @@ export const updateAgentConfiguration = async ({
669669
parseEnv = parseEnvironmentFile,
670670
adapterRegistry = { getAdapter, listAdapterNames },
671671
}) => {
672-
if (!record?.podId || !record?.agentName) {
673-
throw new Error('token record is missing podId/agentName — re-attach the agent');
672+
if (!record?.agentName) {
673+
throw new Error('token record is missing agentName — re-attach the agent');
674674
}
675675
const instanceId = record.instanceId || 'default';
676676
const runtime = {};
@@ -730,13 +730,23 @@ export const updateAgentConfiguration = async ({
730730
throw new Error('provide at least one of --adapter, --model, --effort, or --env');
731731
}
732732

733+
// The token's pod may be an attention room rather than an installation.
734+
// Reuse Settings' owner-filtered projection instead of guessing from it.
735+
const settings = await client.get(
736+
`/api/registry/agents/${encodeURIComponent(record.agentName)}/manage-settings?instanceId=${encodeURIComponent(instanceId)}`,
737+
);
738+
const podId = settings?.runtimeManaged
739+
? settings.runtimeSourcePodId : settings?.writablePodId;
740+
if (typeof podId !== 'string' || !podId.trim()) {
741+
throw new Error('No writable runtime installation found for this agent; check its Settings and computer binding.');
742+
}
733743
await client.patch(
734-
`/api/registry/pods/${record.podId}/agents/${record.agentName}`,
744+
`/api/registry/pods/${encodeURIComponent(podId)}/agents/${encodeURIComponent(record.agentName)}`,
735745
{ instanceId, config },
736746
);
737747
return {
738748
agentName: record.agentName,
739-
podId: record.podId,
749+
podId,
740750
instanceId,
741751
changed: Object.keys(config),
742752
...(runtime.adapter ? { adapter: runtime.adapter } : {}),

0 commit comments

Comments
 (0)