From 4e5e2dfecc842b3d77746bae47491ce57a51dfab Mon Sep 17 00:00:00 2001 From: John Smith Date: Thu, 24 Sep 2026 01:37:40 -0400 Subject: [PATCH] test(ai): cover role resolution and role predicates --- AUTONOMOUS_STATE.json | 8 ++- .../Studio/FinalizeSubAgents.spec.js | 71 +++++++++++++++++++ .../app/src/AiGeneration/Studio/Roles.spec.js | 30 ++++++++ 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/AUTONOMOUS_STATE.json b/AUTONOMOUS_STATE.json index b11defe03..c549c593e 100644 --- a/AUTONOMOUS_STATE.json +++ b/AUTONOMOUS_STATE.json @@ -7,7 +7,7 @@ "asset-curator", "qa-playtester" ], - "last_cycle": 180, + "last_cycle": 181, "completed_targets": [ { "cycle": 75, @@ -1054,6 +1054,12 @@ "target": "Guard the ChatMessages render loops against persisted output holes", "metric_delta": "Three unguarded inner loops in ChatMessages' renderItems built the chat view and read message.type / messageContent.type over a persisted (load-unvalidated) output: a single hole crashed the whole chat render. All three now skip non-objects. Verified by the required production build (exit 0, no ChatMessages warning), eslint --max-warnings=0, prettier, and the full 1601-test suite; component-level render tests remain infeasible (ChatMessages needs the provider tree + ~15 props), so the guards rest on the class already reproduced and pinned in the sibling pure helpers (AiRequestUtils, CustomAIClient). Backlog refreshed: the reliability sweep items are done; remaining entries are the blocked WebUI audit and strategic upstream options.", "commit": "fix(ai): guard the ChatMessages render loops against output holes" + }, + { + "cycle": 181, + "target": "Cover the security-relevant role resolution and role predicates (Tier 1)", + "metric_delta": "The export-vs-spec sweep found getSubAgentRoleId (which decides whether a child is a studio sub-agent and thus its tool subset) and the isStudioRoleId/isSpawnableRoleId predicates had no direct coverage. +6 tests: role resolution from the launching call; hosted-child rejection; null for top-level/missing-parent/unlaunched; predicate truth tables incl. prototype keys and the manager-not-spawnable rule. The hosted test initially did NOT discriminate the isSpawnAgentCall gate (malformed args rejected the role anyway), so it was strengthened with valid spawn-shaped arguments - mutation-checked: removing the gate makes it return 'developer'; restoring it passes. 1606/1606 across 80 suites (was 1601), eslint+prettier clean, divergence clean.", + "commit": "test(ai): cover role resolution and role predicates" } ], "backlog": [ diff --git a/newIDE/app/src/AiGeneration/Studio/FinalizeSubAgents.spec.js b/newIDE/app/src/AiGeneration/Studio/FinalizeSubAgents.spec.js index dbc07f615..ff1b7a223 100644 --- a/newIDE/app/src/AiGeneration/Studio/FinalizeSubAgents.spec.js +++ b/newIDE/app/src/AiGeneration/Studio/FinalizeSubAgents.spec.js @@ -5,6 +5,7 @@ import { getSubAgentReportLabel, countAssistantTurns, isSubAgentAtTurnCap, + getSubAgentRoleId, parentHasOutputForCall, truncateReport, MAX_SUB_AGENT_REPORT_LENGTH, @@ -677,6 +678,76 @@ describe('FinalizeSubAgents', () => { }); }); +describe('getSubAgentRoleId', () => { + // Security-relevant: this decides whether a child is a studio sub-agent (and + // therefore which tool subset it may call). A wrong answer hands a hosted + // child the BYOK role policy, or skips the read-only restriction. + const parentWith = (entries: Array): any => ({ + id: 'parent-1', + output: [assistantMessage('delegating', entries)], + }); + const child = (id: string, parentId: string | null): any => ({ + id, + parentAiRequestId: parentId, + }); + + it('resolves the role from the launching spawn call', () => { + expect( + getSubAgentRoleId({ + aiRequest: child('child-1', 'parent-1'), + aiRequests: { + 'parent-1': parentWith([spawnCall('call-1', 'child-1', null)]), + }, + }) + ).toBe('developer'); + }); + + it('returns null for a hosted child (run_edit_agent is not spawn_agent)', () => { + // Deliberately VALID spawn-shaped arguments: without the isSpawnAgentCall + // gate the role would resolve, so this pins the gate itself (not merely + // malformed-argument rejection). + const hostedCall = { + type: 'function_call', + status: 'completed', + call_id: 'call-1', + name: 'run_edit_agent', + arguments: JSON.stringify({ + role: 'developer', + short_title: 'Build', + task: 'Build it.', + }), + subAgentAiRequestId: 'child-1', + }; + expect( + getSubAgentRoleId({ + aiRequest: child('child-1', 'parent-1'), + aiRequests: { 'parent-1': parentWith([hostedCall]) }, + }) + ).toBeNull(); + }); + + it('returns null for a top-level, missing-parent or unlaunched child', () => { + expect( + getSubAgentRoleId({ + aiRequest: child('child-1', null), + aiRequests: {}, + }) + ).toBeNull(); + expect( + getSubAgentRoleId({ + aiRequest: child('child-1', 'parent-1'), + aiRequests: {}, + }) + ).toBeNull(); + expect( + getSubAgentRoleId({ + aiRequest: child('child-1', 'parent-1'), + aiRequests: { 'parent-1': parentWith([]) }, + }) + ).toBeNull(); + }); +}); + describe('parentHasOutputForCall', () => { // The studio runtime checks this on every pass before finalizing a child: // a null hole in the parent output threw out of the hook effect, killing diff --git a/newIDE/app/src/AiGeneration/Studio/Roles.spec.js b/newIDE/app/src/AiGeneration/Studio/Roles.spec.js index 79185687a..4d4beefce 100644 --- a/newIDE/app/src/AiGeneration/Studio/Roles.spec.js +++ b/newIDE/app/src/AiGeneration/Studio/Roles.spec.js @@ -5,6 +5,9 @@ import { MUTATING_TOOL_NAMES, getStudioRole, getToolsForRole, + isStudioRoleId, + isSpawnableRoleId, + SPAWNABLE_ROLE_IDS, } from './Roles'; import { GDEVELOP_OPENAI_TOOLS } from '../../AI/CustomAIClient'; @@ -46,6 +49,33 @@ describe('Studio roles', () => { }); }); + it('isStudioRoleId accepts every declared role and nothing else', () => { + roleIds.forEach(roleId => expect(isStudioRoleId(roleId)).toBe(true)); + // Prototype keys must not count as roles (hasOwnProperty guard). + expect(isStudioRoleId('toString')).toBe(false); + expect(isStudioRoleId('constructor')).toBe(false); + expect(isStudioRoleId('')).toBe(false); + expect(isStudioRoleId(null)).toBe(false); + expect(isStudioRoleId(42)).toBe(false); + expect(isStudioRoleId({})).toBe(false); + }); + + it('isSpawnableRoleId excludes the manager and non-roles', () => { + expect(SPAWNABLE_ROLE_IDS.slice().sort()).toEqual([ + 'designer', + 'developer', + 'tester', + ]); + // The manager plans; it is a role but never spawned as a sub-agent. + expect(isStudioRoleId('manager')).toBe(true); + expect(isSpawnableRoleId('manager')).toBe(false); + ['designer', 'developer', 'tester'].forEach(roleId => + expect(isSpawnableRoleId(roleId)).toBe(true) + ); + expect(isSpawnableRoleId('architect')).toBe(false); + expect(isSpawnableRoleId(undefined)).toBe(false); + }); + it('throws on an unknown role', () => { expect(() => getStudioRole(('architect': any))).toThrow( 'Unknown studio role'