From bf6320d1d4b78121ad29046a4354f64090bb0a52 Mon Sep 17 00:00:00 2001 From: "[._.]/ Adam Eivy" Date: Tue, 1 Sep 2026 13:37:26 -0700 Subject: [PATCH] fix: require managed app targets for PR reviews --- server/services/cosTaskGenerator.js | 7 +++++ server/services/cosTaskGenerator.test.js | 10 ++++++- server/services/taskSchedule.js | 9 ++++++- server/services/taskSchedule.test.js | 29 +++++++++++++++++++++ server/services/taskScheduleModules.test.js | 3 ++- server/services/taskScheduleRegistry.js | 10 +++++++ 6 files changed, 65 insertions(+), 3 deletions(-) diff --git a/server/services/cosTaskGenerator.js b/server/services/cosTaskGenerator.js index 1c142fe8e1..ae497b3ac3 100644 --- a/server/services/cosTaskGenerator.js +++ b/server/services/cosTaskGenerator.js @@ -1837,6 +1837,13 @@ export async function generateSelfImprovementTaskForType(taskType, state) { const taskSchedule = await import('./taskSchedule.js'); const { getTaskPrompt } = await import('./taskPromptService.js'); const interval = await taskSchedule.getTaskInterval(taskType); + // App-scoped task types must never fall through this global lane. The + // on-demand request gate normally rejects a missing appId, but scheduled + // rotation and older callers can still reach the generator directly. + if (taskSchedule.requiresManagedAppTarget(taskType)) { + emitLog('warn', `Skipping ${taskType} without a managed app target`); + return null; + } let description = await getTaskPrompt(taskType); const metadata = { diff --git a/server/services/cosTaskGenerator.test.js b/server/services/cosTaskGenerator.test.js index 1cf7085e7d..387bb27a46 100644 --- a/server/services/cosTaskGenerator.test.js +++ b/server/services/cosTaskGenerator.test.js @@ -273,7 +273,7 @@ describe('isConfiguredApprovalRequired', () => { it('both generators stamp approvalReason onto metadata so the hint survives COS-TASKS.md', () => { const selfStart = GEN_SRC.indexOf('export async function generateSelfImprovementTaskForType'); const appStart = GEN_SRC.indexOf('export async function generateManagedAppImprovementTaskForType'); - expect(GEN_SRC.slice(selfStart, selfStart + 4500)).toContain('stampApprovalReason(metadata, approval)'); + expect(GEN_SRC.slice(selfStart, appStart)).toContain('stampApprovalReason(metadata, approval)'); expect(GEN_SRC.slice(appStart, appStart + 12000)).toContain('stampApprovalReason(metadata, approval)'); }); @@ -1746,6 +1746,14 @@ describe('the drain cap has exactly one implementation, at the choke point', () }); describe('pr-reviewer security preflight wiring', () => { + it('does not allow the global generator to bypass the managed-app target boundary', () => { + const start = GEN_SRC.indexOf('export async function generateSelfImprovementTaskForType'); + const body = GEN_SRC.slice(start, start + 1800); + expect(body).toContain('taskSchedule.requiresManagedAppTarget(taskType)'); + expect(body).toContain('Skipping ${taskType} without a managed app target'); + expect(body).toContain('return null;'); + }); + it('runs the direct preflight before stage gates and resolves the next-stage prompt', () => { const start = GEN_SRC.indexOf('export async function generateManagedAppImprovementTaskForType'); const body = GEN_SRC.slice(start, GEN_SRC.indexOf('return task;', start)); diff --git a/server/services/taskSchedule.js b/server/services/taskSchedule.js index cf95a6edba..a0bfb27bc6 100644 --- a/server/services/taskSchedule.js +++ b/server/services/taskSchedule.js @@ -40,10 +40,12 @@ import { import { DEFAULT_TASK_INTERVALS, INSTALL_WIDE_TASK_TYPES, + MANAGED_APP_TARGET_TASK_TYPES, MANAGED_AGENT_OPTIONS, getTaskTypeDescription, getTaskTypeInvocation, getTaskTypePromptInfo, + requiresManagedAppTarget, enforceBranchReconcileBatch, enforceManagedAgentOptions } from './taskScheduleRegistry.js'; @@ -64,9 +66,11 @@ export { } from './taskScheduleConstants.js'; export { DEFAULT_BRANCHES_PER_AGENT, DEFAULT_TASK_INTERVALS, INSTALL_WIDE_TASK_TYPES, + MANAGED_APP_TARGET_TASK_TYPES, MANAGED_AGENT_OPTIONS, PERPETUAL_DRAIN_DISPATCH_CAP, SELF_IMPROVEMENT_TASK_TYPES, TASK_TYPE_DESCRIPTIONS, TASK_TYPE_INVOCATION, TASK_TYPE_PROMPT_INFO, - getTaskTypeInvocation, getTaskTypePromptInfo, stripManagedAgentOptionsFromOverride + getTaskTypeInvocation, getTaskTypePromptInfo, requiresManagedAppTarget, + stripManagedAgentOptionsFromOverride } from './taskScheduleRegistry.js'; export { loadSchedule } from './taskScheduleStore.js'; export { @@ -1149,6 +1153,9 @@ export async function triggerOnDemandTask(taskType, appId = null, { emit = true, if (origin === ON_DEMAND_ORIGINS.USER && !invocation.userInvokable) { return { result: { error: `Task type '${taskType}' is managed by another automation and cannot be run manually` }, changed: false }; } + if (requiresManagedAppTarget(taskType) && !appId) { + return { result: { error: `Task type '${taskType}' requires a managed app target` }, changed: false }; + } // Reject if the master Improve toggle is off — request would be silently dropped downstream const state = await loadState(); diff --git a/server/services/taskSchedule.test.js b/server/services/taskSchedule.test.js index 5283f3232a..1c30b9d729 100644 --- a/server/services/taskSchedule.test.js +++ b/server/services/taskSchedule.test.js @@ -151,12 +151,14 @@ import { FAILURE_PARK_THRESHOLD, PROMPT_VERSIONS, DEFAULT_TASK_INTERVALS, + MANAGED_APP_TARGET_TASK_TYPES, MANAGED_AGENT_OPTIONS, stripManagedAgentOptionsFromOverride, TASK_TYPE_DESCRIPTIONS, TASK_TYPE_INVOCATION, TASK_TYPE_PROMPT_INFO, getTaskTypeInvocation, + requiresManagedAppTarget, REFERENCE_WATCH_AUDITED_VERSION, boundParkedUntil } from './taskSchedule.js' @@ -284,6 +286,21 @@ describe('taskSchedule', () => { }) }) + describe('managed-app target task types', () => { + it('keeps app-required scope explicit and separate from install-wide scope', () => { + expect([...MANAGED_APP_TARGET_TASK_TYPES]).toEqual(['pr-reviewer']) + expect(requiresManagedAppTarget('pr-reviewer')).toBe(true) + expect(requiresManagedAppTarget('security')).toBe(false) + expect(requiresManagedAppTarget('repo-sync')).toBe(false) + }) + + it('only names registered task types', () => { + for (const taskType of MANAGED_APP_TARGET_TASK_TYPES) { + expect(SELF_IMPROVEMENT_TASK_TYPES).toContain(taskType) + } + }) + }) + describe('TASK_TYPE_DESCRIPTIONS', () => { // Guards against the "orphaned task" bug: a task type with no description // entry falls back to a dasherized label ("claim work") in the schedule UI, @@ -1930,6 +1947,18 @@ describe('taskSchedule', () => { expect(result.origin).toBe(ON_DEMAND_ORIGINS.USER) }) + it('rejects an app-required task without a managed app target', async () => { + mockSchedule({ + tasks: { 'pr-reviewer': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true } } + }) + + const result = await triggerOnDemandTask('pr-reviewer') + + expect(result.error).toMatch(/requires a managed app target/i) + expect(recordUserAction).not.toHaveBeenCalled() + expect((await getOnDemandRequests()).filter(r => r.taskType === 'pr-reviewer')).toHaveLength(0) + }) + it('should reject unknown task types instead of silently queuing them', async () => { mockSchedule({ tasks: { 'feature-ideas': { type: 'weekly', enabled: true } } diff --git a/server/services/taskScheduleModules.test.js b/server/services/taskScheduleModules.test.js index a1fd7c7150..a88d96438f 100644 --- a/server/services/taskScheduleModules.test.js +++ b/server/services/taskScheduleModules.test.js @@ -14,9 +14,10 @@ describe('taskSchedule module boundaries', () => { ...['INTERVAL_TYPES', 'ON_DEMAND_ORIGINS', 'isRefillRequest'] .map((name) => [name, constants[name]]), ...['DEFAULT_BRANCHES_PER_AGENT', 'DEFAULT_TASK_INTERVALS', 'MANAGED_AGENT_OPTIONS', + 'MANAGED_APP_TARGET_TASK_TYPES', 'PERPETUAL_DRAIN_DISPATCH_CAP', 'SELF_IMPROVEMENT_TASK_TYPES', 'TASK_TYPE_DESCRIPTIONS', 'TASK_TYPE_INVOCATION', 'TASK_TYPE_PROMPT_INFO', 'getTaskTypeInvocation', 'getTaskTypePromptInfo', - 'stripManagedAgentOptionsFromOverride'].map((name) => [name, registry[name]]), + 'requiresManagedAppTarget', 'stripManagedAgentOptionsFromOverride'].map((name) => [name, registry[name]]), ...['FAILURE_BACKOFF_BASE_MS', 'FAILURE_BACKOFF_CAP_MS', 'FAILURE_PARK_THRESHOLD', 'clearTaskTypeFailurePark', 'computeFailureBackoffMs', 'recordTaskTypeFailure', 'recordTaskTypeSuccess'].map((name) => [name, backoff[name]]), diff --git a/server/services/taskScheduleRegistry.js b/server/services/taskScheduleRegistry.js index 3c473e93e3..30e2d02037 100644 --- a/server/services/taskScheduleRegistry.js +++ b/server/services/taskScheduleRegistry.js @@ -200,6 +200,16 @@ export const DEFAULT_BRANCHES_PER_AGENT = 3; */ export const INSTALL_WIDE_TASK_TYPES = new Set(['repo-sync', 'user-action-review']); +// Task types that only make sense when pointed at a managed app. Keeping this +// alongside the install-wide registry gives both the on-demand request gate +// and the global generator one target-scope contract; neither has to infer +// scope from a task name or from which generator happened to receive a call. +export const MANAGED_APP_TARGET_TASK_TYPES = new Set(['pr-reviewer']); + +export function requiresManagedAppTarget(taskType) { + return MANAGED_APP_TARGET_TASK_TYPES.has(taskType); +} + // Fresh installs expose every task as an enabled manual action. The on-demand // type keeps provider work silent until the user explicitly runs a task, while // retaining timing metadata such as custom intervals and recheck settings if