fix(team): complete teammate permission responses
This commit is contained in:
parent
471fe0a1a8
commit
be558927ea
4 changed files with 229 additions and 86 deletions
|
|
@ -31999,103 +31999,100 @@ export class TeamProvisioningService {
|
|||
});
|
||||
};
|
||||
|
||||
// Apply permission_suggestions: add tool rules to project settings file
|
||||
// Apply permission_suggestions: add tool rules to project settings file.
|
||||
if (suggestions.length === 0) {
|
||||
sendSuccessResponse();
|
||||
logger.info(
|
||||
`[${run.teamName}] No permission_suggestions for ${requestId}; inbox response sent`
|
||||
`[${run.teamName}] No permission_suggestions for ${requestId}; sending allow responses only`
|
||||
);
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
// Resolve project cwd from team config
|
||||
let projectCwd: string | undefined;
|
||||
try {
|
||||
const config = await this.readConfigForStrictDecision(run.teamName);
|
||||
projectCwd = config?.projectPath ?? config?.members?.[0]?.cwd;
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
|
||||
// Resolve project cwd from team config
|
||||
let projectCwd: string | undefined;
|
||||
try {
|
||||
const config = await this.readConfigForStrictDecision(run.teamName);
|
||||
projectCwd = config?.projectPath ?? config?.members?.[0]?.cwd;
|
||||
} catch {
|
||||
// best-effort
|
||||
}
|
||||
if (!projectCwd) {
|
||||
logger.warn(
|
||||
`[${run.teamName}] Cannot resolve project cwd for permission rule; sending inbox response only`
|
||||
);
|
||||
sendSuccessResponse();
|
||||
return;
|
||||
}
|
||||
if (!projectCwd) {
|
||||
logger.warn(
|
||||
`[${run.teamName}] Cannot resolve project cwd for permission rule; sending allow responses only`
|
||||
);
|
||||
} else {
|
||||
for (const suggestion of suggestions) {
|
||||
// Handle "setMode" suggestions (e.g. Write/Edit tools suggest acceptEdits mode)
|
||||
// FACT: Write/Edit permission_requests have permission_suggestions:
|
||||
// { type: "setMode", mode: "acceptEdits", destination: "session" }
|
||||
// Since we can't change session mode of a subprocess, we translate to addRules.
|
||||
if (suggestion.type === 'setMode') {
|
||||
const mode = typeof suggestion.mode === 'string' ? suggestion.mode : '';
|
||||
let toolNames: string[] = [];
|
||||
if (mode === 'acceptEdits') {
|
||||
toolNames = ['Edit', 'Write', 'NotebookEdit'];
|
||||
} else if (mode === 'bypassPermissions') {
|
||||
// Broad approval - add common tools
|
||||
toolNames = ['Edit', 'Write', 'NotebookEdit', 'Bash', 'Read', 'Grep', 'Glob'];
|
||||
}
|
||||
if (toolNames.length > 0) {
|
||||
const settingsPath = path.join(projectCwd, '.claude', 'settings.local.json');
|
||||
try {
|
||||
await this.addPermissionRulesToSettings(settingsPath, toolNames, 'allow');
|
||||
logger.info(
|
||||
`[${run.teamName}] Applied setMode "${mode}" for ${agentId}: ${toolNames.join(', ')} in ${settingsPath}`
|
||||
);
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`[${run.teamName}] Failed to apply setMode: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`
|
||||
);
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (suggestion.type !== 'addRules' || !Array.isArray(suggestion.rules)) continue;
|
||||
|
||||
let toolNames = suggestion.rules
|
||||
.map((r) => r.toolName)
|
||||
.filter((name): name is string => typeof name === 'string' && name.length > 0);
|
||||
if (toolNames.length === 0) continue;
|
||||
|
||||
// Expand teammate-safe operational tools only.
|
||||
// This removes the bootstrap/task workflow race without accidentally granting
|
||||
// admin/runtime tools like team_stop or kanban_clear.
|
||||
if (
|
||||
toolNames.some((name) =>
|
||||
AGENT_TEAMS_NAMESPACED_TEAMMATE_OPERATIONAL_TOOL_NAMES.includes(name)
|
||||
)
|
||||
) {
|
||||
const merged = new Set([
|
||||
...toolNames,
|
||||
...AGENT_TEAMS_NAMESPACED_TEAMMATE_OPERATIONAL_TOOL_NAMES,
|
||||
]);
|
||||
toolNames = Array.from(merged);
|
||||
}
|
||||
|
||||
const behavior = suggestion.behavior ?? 'allow';
|
||||
// FACT: observed destinations are "localSettings" (project-level .claude/settings.local.json)
|
||||
const settingsPath =
|
||||
suggestion.destination === 'localSettings'
|
||||
? path.join(projectCwd, '.claude', 'settings.local.json')
|
||||
: path.join(projectCwd, '.claude', 'settings.local.json'); // default to local
|
||||
|
||||
for (const suggestion of suggestions) {
|
||||
// Handle "setMode" suggestions (e.g. Write/Edit tools suggest acceptEdits mode)
|
||||
// FACT: Write/Edit permission_requests have permission_suggestions:
|
||||
// { type: "setMode", mode: "acceptEdits", destination: "session" }
|
||||
// Since we can't change session mode of a subprocess, we translate to addRules.
|
||||
if (suggestion.type === 'setMode') {
|
||||
const mode = typeof suggestion.mode === 'string' ? suggestion.mode : '';
|
||||
let toolNames: string[] = [];
|
||||
if (mode === 'acceptEdits') {
|
||||
toolNames = ['Edit', 'Write', 'NotebookEdit'];
|
||||
} else if (mode === 'bypassPermissions') {
|
||||
// Broad approval - add common tools
|
||||
toolNames = ['Edit', 'Write', 'NotebookEdit', 'Bash', 'Read', 'Grep', 'Glob'];
|
||||
}
|
||||
if (toolNames.length > 0) {
|
||||
const settingsPath = path.join(projectCwd, '.claude', 'settings.local.json');
|
||||
try {
|
||||
await this.addPermissionRulesToSettings(settingsPath, toolNames, 'allow');
|
||||
await this.addPermissionRulesToSettings(settingsPath, toolNames, behavior);
|
||||
logger.info(
|
||||
`[${run.teamName}] Applied setMode "${mode}" for ${agentId}: ${toolNames.join(', ')} in ${settingsPath}`
|
||||
`[${run.teamName}] Added permission rules for ${agentId}: ${toolNames.join(', ')} -> ${behavior} in ${settingsPath}`
|
||||
);
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`[${run.teamName}] Failed to apply setMode: ${
|
||||
`[${run.teamName}] Failed to add permission rules: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`
|
||||
);
|
||||
}
|
||||
}
|
||||
continue;
|
||||
}
|
||||
|
||||
if (suggestion.type !== 'addRules' || !Array.isArray(suggestion.rules)) continue;
|
||||
|
||||
let toolNames = suggestion.rules
|
||||
.map((r) => r.toolName)
|
||||
.filter((name): name is string => typeof name === 'string' && name.length > 0);
|
||||
if (toolNames.length === 0) continue;
|
||||
|
||||
// Expand teammate-safe operational tools only.
|
||||
// This removes the bootstrap/task workflow race without accidentally granting
|
||||
// admin/runtime tools like team_stop or kanban_clear.
|
||||
if (
|
||||
toolNames.some((name) =>
|
||||
AGENT_TEAMS_NAMESPACED_TEAMMATE_OPERATIONAL_TOOL_NAMES.includes(name)
|
||||
)
|
||||
) {
|
||||
const merged = new Set([
|
||||
...toolNames,
|
||||
...AGENT_TEAMS_NAMESPACED_TEAMMATE_OPERATIONAL_TOOL_NAMES,
|
||||
]);
|
||||
toolNames = Array.from(merged);
|
||||
}
|
||||
|
||||
const behavior = suggestion.behavior ?? 'allow';
|
||||
// FACT: observed destinations are "localSettings" (project-level .claude/settings.local.json)
|
||||
const settingsPath =
|
||||
suggestion.destination === 'localSettings'
|
||||
? path.join(projectCwd, '.claude', 'settings.local.json')
|
||||
: path.join(projectCwd, '.claude', 'settings.local.json'); // default to local
|
||||
|
||||
try {
|
||||
await this.addPermissionRulesToSettings(settingsPath, toolNames, behavior);
|
||||
logger.info(
|
||||
`[${run.teamName}] Added permission rules for ${agentId}: ${toolNames.join(', ')} -> ${behavior} in ${settingsPath}`
|
||||
);
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`[${run.teamName}] Failed to add permission rules: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -32186,7 +32183,7 @@ export class TeamProvisioningService {
|
|||
message: string | undefined
|
||||
): Record<string, unknown> | undefined {
|
||||
if (!toolInput) return undefined;
|
||||
if (toolName !== 'AskUserQuestion' || !message) return toolInput;
|
||||
if (toolName !== 'AskUserQuestion' || message === undefined) return toolInput;
|
||||
|
||||
const answers = this.parseAskUserQuestionAnswers(message, toolInput);
|
||||
return Object.keys(answers).length > 0 ? { ...toolInput, answers } : toolInput;
|
||||
|
|
|
|||
|
|
@ -147,6 +147,34 @@ describe('team model availability Codex catalog integration', () => {
|
|||
availabilityStatus: 'available',
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.3-codex-spark',
|
||||
label: '5.3 Codex Spark',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.2-codex',
|
||||
label: '5.2 Codex',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.1-codex-mini',
|
||||
label: '5.1 Codex Mini',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.1-codex-max',
|
||||
label: '5.1 Codex Max',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -224,7 +252,16 @@ describe('team model availability Codex catalog integration', () => {
|
|||
|
||||
expect(
|
||||
getAvailableTeamProviderModelOptions('codex', providerStatus).map((model) => model.value)
|
||||
).toEqual(['', 'gpt-5.5', 'gpt-5.4', 'gpt-5.2']);
|
||||
).toEqual([
|
||||
'',
|
||||
'gpt-5.5',
|
||||
'gpt-5.4',
|
||||
'gpt-5.3-codex-spark',
|
||||
'gpt-5.2',
|
||||
'gpt-5.2-codex',
|
||||
'gpt-5.1-codex-mini',
|
||||
'gpt-5.1-codex-max',
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps existing disabled model policy on top of the dynamic catalog', () => {
|
||||
|
|
|
|||
|
|
@ -14454,6 +14454,26 @@ describe('TeamProvisioningService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('preserves blank teammate AskUserQuestion answers', () => {
|
||||
const svc = new TeamProvisioningService();
|
||||
const toolInput = {
|
||||
questions: [
|
||||
{
|
||||
question: 'Anything else?',
|
||||
options: [{ label: 'Skip', description: 'No extra details' }],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
expect((svc as any).buildTeammatePermissionUpdatedInput('AskUserQuestion', toolInput, ''))
|
||||
.toEqual({
|
||||
...toolInput,
|
||||
answers: {
|
||||
'Anything else?': '',
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('sends teammate AskUserQuestion permission responses to the teammate inbox', async () => {
|
||||
const svc = new TeamProvisioningService();
|
||||
const persistInboxMessage = vi.fn();
|
||||
|
|
@ -14570,6 +14590,45 @@ describe('TeamProvisioningService', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('sends teammate fallback control responses without permission suggestions', async () => {
|
||||
const write = vi.fn((_line: string, cb?: (error?: Error | null) => void) => {
|
||||
cb?.();
|
||||
return true;
|
||||
});
|
||||
const svc = new TeamProvisioningService();
|
||||
(svc as any).persistInboxMessage = vi.fn();
|
||||
const toolInput = {
|
||||
questions: [
|
||||
{
|
||||
question: 'Anything else?',
|
||||
options: [{ label: 'Skip', description: 'No extra details' }],
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
await (svc as any).respondToTeammatePermission(
|
||||
{
|
||||
teamName: 'ops-team',
|
||||
runId: 'run-1',
|
||||
child: { stdin: { writable: true, write } },
|
||||
},
|
||||
'bob',
|
||||
'perm-3',
|
||||
true,
|
||||
'',
|
||||
[],
|
||||
'AskUserQuestion',
|
||||
toolInput
|
||||
);
|
||||
|
||||
expect(write).toHaveBeenCalledTimes(1);
|
||||
const payload = JSON.parse(write.mock.calls[0][0]);
|
||||
expect(payload.response.response.updatedInput).toEqual({
|
||||
...toolInput,
|
||||
answers: { 'Anything else?': '' },
|
||||
});
|
||||
});
|
||||
|
||||
it('uses a non-alarming model delay message before 2 minutes of silence', () => {
|
||||
const svc = new TeamProvisioningService();
|
||||
|
||||
|
|
|
|||
|
|
@ -111,18 +111,46 @@ describe('teamModelAvailability', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('builds Codex model options from the runtime list instead of the hardcoded fallback', () => {
|
||||
it('builds Codex model options from the runtime list plus disabled safety entries', () => {
|
||||
const providerStatus = createCodexProviderStatus(['gpt-5.4', 'gpt-5.3-codex']);
|
||||
|
||||
expect(getAvailableTeamProviderModelOptions('codex', providerStatus)).toEqual([
|
||||
{ value: '', label: 'Default', badgeLabel: 'Default' },
|
||||
{ value: 'gpt-5.4', label: '5.4', availabilityStatus: 'available', availabilityReason: null },
|
||||
{
|
||||
value: 'gpt-5.4',
|
||||
label: '5.4',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: 'available',
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.3-codex',
|
||||
label: '5.3 Codex',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: 'available',
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.3-codex-spark',
|
||||
label: '5.3 Codex Spark',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.2-codex',
|
||||
label: '5.2 Codex',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.1-codex-mini',
|
||||
label: '5.1 Codex Mini',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
@ -148,9 +176,31 @@ describe('teamModelAvailability', () => {
|
|||
{
|
||||
value: 'gpt-5.4',
|
||||
label: '5.4',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: 'unavailable',
|
||||
availabilityReason: 'No access for this account',
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.3-codex-spark',
|
||||
label: '5.3 Codex Spark',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.2-codex',
|
||||
label: '5.2 Codex',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
{
|
||||
value: 'gpt-5.1-codex-mini',
|
||||
label: '5.1 Codex Mini',
|
||||
badgeLabel: undefined,
|
||||
availabilityStatus: null,
|
||||
availabilityReason: null,
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue