Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/services/cosTaskGenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ async function spawnPriority0OnDemand(ctx) {

const taskSchedule = await import('./taskSchedule.js');
const liveSchedule = await taskSchedule.loadSchedule();
const onDemandRequests = Array.isArray(liveSchedule?.onDemandRequests) ? liveSchedule.onDemandRequests : [];
const onDemandRequests = await taskSchedule.getOnDemandRequests();

// Track apps already marked review-started this cycle so multiple on-demand
// requests for the same app don't each rewrite its activity record.
Expand Down
64 changes: 50 additions & 14 deletions server/services/taskSchedule.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ import {
enforceManagedAgentOptions
} from './taskScheduleRegistry.js';
import { loadSchedule, updateSchedule } from './taskScheduleStore.js';
import { isInstanceFeatureEnabled } from './instanceFeatures.js';
import { getTaskDataInputCatalog } from '../lib/taskDataInputCatalog.js';
import {
clearFailureLedgerFields,
Expand All @@ -71,6 +72,18 @@ export {
} from './taskScheduleBackoff.js';
export { addTemplateTask, deleteTemplateTask, getTemplateTasks } from './taskScheduleTemplates.js';

const createFeatureGate = () => {
const enabledByFeature = new Map();
return async (interval) => {
const featureId = interval?.feature;
if (!featureId) return true;
if (!enabledByFeature.has(featureId)) {
enabledByFeature.set(featureId, isInstanceFeatureEnabled(featureId));
}
return enabledByFeature.get(featureId);
};
};

/**
* Get learning-adjusted interval for a task type
*/
Expand Down Expand Up @@ -717,7 +730,7 @@ export async function applyOnDemandRunResets(request, appId = null) {
* not run automatically and would otherwise block the dependent task indefinitely.
* A scheduled per-app override keeps its dependency gate active.
*/
async function checkRunAfterDeps(schedule, taskType, appId = null) {
async function checkRunAfterDeps(schedule, taskType, appId = null, featureEnabled = createFeatureGate()) {
const interval = schedule.tasks[taskType];
const deps = interval?.runAfter;
if (!deps || deps.length === 0) return { satisfied: true, pending: [] };
Expand All @@ -730,6 +743,7 @@ async function checkRunAfterDeps(schedule, taskType, appId = null) {
for (const dep of deps) {
const depConfig = schedule.tasks[dep];
if (!depConfig || !depConfig.enabled) continue;
if (!(await featureEnabled(depConfig))) continue;
if (appId && !(await isTaskTypeEnabledForApp(appId, dep))) continue;
const depPerAppInterval = appId ? await getAppTaskTypeInterval(appId, dep) : null;
if ((depPerAppInterval || depConfig.type) === INTERVAL_TYPES.ON_DEMAND) continue;
Expand Down Expand Up @@ -770,13 +784,16 @@ async function evaluateFixedInterval(taskType, baseIntervalMs, label, timeSinceL
/**
* Check if a task type should run for a specific app (or globally)
*/
export async function shouldRunTask(taskType, appId = null) {
export async function shouldRunTask(taskType, appId = null, { featureEnabled = createFeatureGate() } = {}) {
const schedule = await loadSchedule();
const interval = schedule.tasks[taskType];

if (!interval || !interval.enabled) {
return { shouldRun: false, reason: 'disabled' };
}
if (!(await featureEnabled(interval))) {
return { shouldRun: false, reason: 'feature-disabled', feature: interval.feature };
}

// Fetch timezone once for reuse across weekday and cron checks
const timezone = await getUserTimezone();
Expand Down Expand Up @@ -995,7 +1012,7 @@ export async function shouldRunTask(taskType, appId = null) {
// If the task would run, check runAfter dependencies — blocked until all enabled deps have run since our last run.
// Disabled deps (globally or for this app) are skipped, since they'll never run.
if (result.shouldRun && interval.runAfter?.length > 0) {
const depCheck = await checkRunAfterDeps(schedule, taskType, appId);
const depCheck = await checkRunAfterDeps(schedule, taskType, appId, featureEnabled);
if (!depCheck.satisfied) {
return { shouldRun: false, reason: 'waiting-on-dependencies', pendingDeps: depCheck.pending };
}
Expand All @@ -1010,11 +1027,12 @@ export async function shouldRunTask(taskType, appId = null) {
export async function getDueTasks(appId = null) {
const schedule = await loadSchedule();
const due = [];
const featureEnabled = createFeatureGate();

for (const [taskType, interval] of Object.entries(schedule.tasks)) {
if (!interval.enabled) continue;

const check = await shouldRunTask(taskType, appId);
const check = await shouldRunTask(taskType, appId, { featureEnabled });
if (check.shouldRun) {
due.push({ taskType, reason: check.reason, interval });
}
Expand All @@ -1028,8 +1046,6 @@ export async function getDueTasks(appId = null) {
*/
export async function getNextTaskType(appId = null, lastType = '', { perpetualOnly = false } = {}) {
const schedule = await loadSchedule();
const taskTypes = Object.keys(schedule.tasks);

const dueTasks = await getDueTasks(appId);

// `perpetualOnly` constrains the pick to a due perpetual (drain-until-done)
Expand Down Expand Up @@ -1082,10 +1098,13 @@ export async function getNextTaskType(appId = null, lastType = '', { perpetualOn
}

// Fall back to rotation among enabled rotation tasks
const rotationTasks = taskTypes.filter(t =>
schedule.tasks[t].enabled &&
schedule.tasks[t].type === INTERVAL_TYPES.ROTATION
);
const featureEnabled = createFeatureGate();
const rotationTasks = [];
for (const [taskType, interval] of Object.entries(schedule.tasks)) {
if (interval.enabled && interval.type === INTERVAL_TYPES.ROTATION && await featureEnabled(interval)) {
rotationTasks.push(taskType);
}
}

if (rotationTasks.length === 0) {
return null;
Expand Down Expand Up @@ -1119,6 +1138,9 @@ export async function triggerOnDemandTask(taskType, appId = null, { emit = true,
if (!tasks[taskType].enabled) {
return { result: { error: `Task type '${taskType}' is disabled` }, changed: false };
}
if (!(await createFeatureGate()(tasks[taskType]))) {
return { result: { error: `Task type '${taskType}' requires the '${tasks[taskType].feature}' feature` }, changed: false };
}

// Reject if the master Improve toggle is off — request would be silently dropped downstream
const state = await loadState();
Expand Down Expand Up @@ -1159,7 +1181,16 @@ export async function triggerOnDemandTask(taskType, appId = null, { emit = true,

export async function getOnDemandRequests() {
const schedule = await loadSchedule();
return schedule.onDemandRequests || [];
const featureEnabled = createFeatureGate();
return getAvailableOnDemandRequests(schedule, featureEnabled);
}

async function getAvailableOnDemandRequests(schedule, featureEnabled) {
const requests = schedule.onDemandRequests || [];
const availability = await Promise.all(requests.map((request) => (
featureEnabled(schedule.tasks?.[request.taskType])
)));
return requests.filter((_request, index) => availability[index]);
}

export async function clearOnDemandRequest(requestId) {
Expand All @@ -1181,13 +1212,15 @@ export async function clearOnDemandRequest(requestId) {
export async function getScheduleStatus() {
// Surface the master Improve toggle so the UI can disable Run Now affordances
const [schedule, state] = await Promise.all([loadSchedule(), loadState()]);
const featureEnabled = createFeatureGate();
const onDemandRequests = await getAvailableOnDemandRequests(schedule, featureEnabled);

const status = {
lastUpdated: schedule.lastUpdated,
improvementEnabled: isImprovementEnabled(state),
tasks: {},
templates: schedule.templates,
onDemandRequests: schedule.onDemandRequests || [],
onDemandRequests,
learningAdjustmentsActive: 0,
dataInputCatalog: getTaskDataInputCatalog()
};
Expand All @@ -1197,14 +1230,15 @@ export async function getScheduleStatus() {
const totalAppCount = activeApps.length;

for (const [taskType, interval] of Object.entries(schedule.tasks)) {
if (!(await featureEnabled(interval))) continue;
const execution = schedule.executions[`task:${taskType}`] || { lastRun: null, count: 0, perApp: {} };

// Get learning adjustment info
const baseInterval = interval.type === 'daily' ? DAY : interval.type === 'weekly' ? WEEK : (interval.intervalMs || DAY);
const learningInfo = await getPerformanceAdjustedInterval(taskType, baseInterval);

// Check global shouldRun status
const check = await shouldRunTask(taskType);
const check = await shouldRunTask(taskType, null, { featureEnabled });

const isEnabledForApp = (override) => override?.enabled === true;
const appOverrides = {};
Expand Down Expand Up @@ -1365,12 +1399,14 @@ export async function getUpcomingTasks(limit = 10) {
const schedule = await loadSchedule();
const now = Date.now();
const upcoming = [];
const featureEnabled = createFeatureGate();

for (const [taskType, interval] of Object.entries(schedule.tasks)) {
if (!interval.enabled) continue;
if (!(await featureEnabled(interval))) continue;
if (interval.type === INTERVAL_TYPES.ON_DEMAND) continue;

const check = await shouldRunTask(taskType);
const check = await shouldRunTask(taskType, null, { featureEnabled });
const execution = schedule.executions[`task:${taskType}`] || { lastRun: null, count: 0 };

let eligibleAt = now;
Expand Down
78 changes: 76 additions & 2 deletions server/services/taskSchedule.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ vi.mock('./apps.js', () => ({
clearAllIssueWatcherState: vi.fn().mockResolvedValue({ changed: false })
}))

vi.mock('./instanceFeatures.js', () => ({
isInstanceFeatureEnabled: vi.fn().mockResolvedValue(true),
}))

vi.mock('../lib/ports.js', () => ({
PORTOS_UI_URL: 'http://localhost:5554',
PORTOS_API_URL: 'http://localhost:5555'
Expand Down Expand Up @@ -121,6 +125,7 @@ import {
deleteTemplateTask,
resetExecutionHistory,
triggerOnDemandTask,
getOnDemandRequests,
getScheduleStatus,
computePerpetualRecheckAt,
parkPerpetual,
Expand Down Expand Up @@ -172,9 +177,10 @@ import { getLocalParts } from '../lib/timezone.js'
import { getAdaptiveCooldownMultiplier } from './taskLearning.js'
import { parseCronToNextRun, parseCronToPrevRun } from './eventScheduler.js'
import { addNotification, exists as notificationExists, removeByMetadata } from './notifications.js'
import { isInstanceFeatureEnabled } from './instanceFeatures.js'

const mockSchedule = ({ tasks = {}, executions = {}, templates = [] } = {}) => {
readJSONFile.mockResolvedValue({ version: 2, tasks, executions, templates })
const mockSchedule = ({ tasks = {}, executions = {}, templates = [], onDemandRequests = [] } = {}) => {
readJSONFile.mockResolvedValue({ version: 2, tasks, executions, templates, onDemandRequests })
}

// Resolve "the most recent 9 AM in the past, local time." Bare
Expand All @@ -193,6 +199,7 @@ const recentNineAm = () => {
describe('taskSchedule', () => {
beforeEach(() => {
vi.clearAllMocks()
isInstanceFeatureEnabled.mockResolvedValue(true)
// Default: no saved schedule → use defaults
readJSONFile.mockResolvedValue(null)
})
Expand Down Expand Up @@ -235,6 +242,25 @@ describe('taskSchedule', () => {
})
})

describe('feature-gated shipped tasks', () => {
it('associates both shipped JIRA tasks with the JIRA instance feature', () => {
expect(DEFAULT_TASK_INTERVALS['jira-sprint-manager'].feature).toBe('jira')
expect(DEFAULT_TASK_INTERVALS['jira-status-report'].feature).toBe('jira')
})

it('keeps the shipped feature association authoritative over persisted state', async () => {
mockSchedule({ tasks: {
'jira-sprint-manager': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, feature: 'post' },
security: { type: INTERVAL_TYPES.WEEKLY, enabled: true, feature: 'jira' },
} })

const schedule = await loadSchedule()

expect(schedule.tasks['jira-sprint-manager'].feature).toBe('jira')
expect(schedule.tasks.security).not.toHaveProperty('feature')
})
})

describe('INSTALL_WIDE_TASK_TYPES', () => {
it('names only task types that really sweep the whole install', () => {
expect([...INSTALL_WIDE_TASK_TYPES]).toEqual(['repo-sync'])
Expand Down Expand Up @@ -946,6 +972,15 @@ describe('taskSchedule', () => {
})

describe('shouldRunTask', () => {
it('does not run a task whose required instance feature is disabled', async () => {
isInstanceFeatureEnabled.mockResolvedValue(false)
mockSchedule({ tasks: { 'jira-sprint-manager': { type: 'rotation', enabled: true } } })

const result = await shouldRunTask('jira-sprint-manager')

expect(result).toEqual({ shouldRun: false, reason: 'feature-disabled', feature: 'jira' })
})

it('should not run disabled task', async () => {
mockSchedule({
tasks: { 'disabled-task': { type: 'weekly', enabled: false, providerId: null, model: null, prompt: null } }
Expand Down Expand Up @@ -1302,6 +1337,15 @@ describe('taskSchedule', () => {
expect(result.taskType).toBe('error-handling')
})

it('does not select a feature-disabled rotation task', async () => {
isInstanceFeatureEnabled.mockResolvedValue(false)
mockSchedule({ tasks: {
'jira-sprint-manager': { type: INTERVAL_TYPES.ROTATION, enabled: true },
} })

expect(await getNextTaskType()).toBeNull()
})

it('prefers a due cron task over a perpetually-ready weekly task', async () => {
// A weekly task with no execution record is perpetually 'ready' (weekly-due).
// A cron task firing right now should still win — explicit time-based schedules
Expand Down Expand Up @@ -1806,6 +1850,26 @@ describe('taskSchedule', () => {
expect(loadState).not.toHaveBeenCalled()
})

it('rejects a manual run when the task feature is disabled', async () => {
isInstanceFeatureEnabled.mockResolvedValue(false)
mockSchedule({ tasks: { 'jira-sprint-manager': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true } } })

const result = await triggerOnDemandTask('jira-sprint-manager', 'app-1')

expect(result.error).toMatch(/requires the 'jira' feature/i)
expect(loadState).not.toHaveBeenCalled()
})

it('does not dispatch a queued request after its task feature is disabled', async () => {
isInstanceFeatureEnabled.mockResolvedValue(false)
mockSchedule({
tasks: { 'jira-sprint-manager': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true } },
onDemandRequests: [{ id: 'demand-existing', taskType: 'jira-sprint-manager', appId: 'app-1' }],
})

expect(await getOnDemandRequests()).toEqual([])
})

it('should accept a manual run for an enabled on-demand task', async () => {
mockSchedule({
tasks: { 'security': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true } }
Expand Down Expand Up @@ -1936,6 +2000,16 @@ describe('taskSchedule', () => {

expect(status.improvementEnabled).toBe(false)
})

it('hides shipped tasks whose required instance feature is disabled', async () => {
isInstanceFeatureEnabled.mockResolvedValue(false)
mockSchedule({ tasks: { 'jira-sprint-manager': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true } } })

const status = await getScheduleStatus()

expect(status.tasks).not.toHaveProperty('jira-sprint-manager')
expect(status.tasks).not.toHaveProperty('jira-status-report')
})
})

describe('perpetual (drain-until-done)', () => {
Expand Down
8 changes: 5 additions & 3 deletions server/services/taskScheduleRegistry.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,9 @@ export const INSTALL_WIDE_TASK_TYPES = new Set(['repo-sync']);
// type keeps provider work silent until the user explicitly runs a task, while
// retaining timing metadata such as custom intervals and recheck settings if
// they later choose a scheduled interval. Existing persisted settings still
// win when a schedule is loaded.
// win when a schedule is loaded. A `feature` association is the exception: it
// is code-owned and makes the task invisible and non-runnable while that
// install-wide feature is disabled.
export const DEFAULT_TASK_INTERVALS = {
'security': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: false } },
'code-quality': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: false } },
Expand Down Expand Up @@ -320,13 +322,13 @@ export const DEFAULT_TASK_INTERVALS = {
'pr-reviewer': { type: INTERVAL_TYPES.ON_DEMAND, intervalMs: 7200000, enabled: true, weekdaysOnly: true, providerId: null, model: null, prompt: null, taskMetadata: { readOnly: true, pipeline: { stages: [{ name: 'Security Scan', promptKey: 'pr-reviewer-security', readOnly: true }, { name: 'Code Review & Merge', promptKey: 'pr-reviewer-review', readOnly: false }] } } },
'code-reviewer-a': { ...CODE_REVIEWER_INTERVAL },
'code-reviewer-b': { ...CODE_REVIEWER_INTERVAL },
'jira-sprint-manager': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, weekdaysOnly: true, providerId: null, model: null, prompt: null, taskMetadata: { useWorktree: true, openPR: true, simplify: true } },
'jira-sprint-manager': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, weekdaysOnly: true, feature: 'jira', providerId: null, model: null, prompt: null, taskMetadata: { useWorktree: true, openPR: true, simplify: true } },
// jira-status-report posts its report to JIRA and edits nothing in the repo, so it
// takes the shared non-committing-coordinator posture above. `readOnly: true` alone
// was NOT enough: it skips worktree creation but leaves `openPR` free to be filled
// from the app's `defaultOpenPR`, and the finalize-time PR-claim check reads
// `metadata.openPR` directly — scoring a posted report as `pr-missing`.
'jira-status-report': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, weekdaysOnly: true, providerId: null, model: null, prompt: null, taskMetadata: { ...NON_COMMITTING_COORDINATOR_METADATA, readOnly: true } },
'jira-status-report': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, weekdaysOnly: true, feature: 'jira', providerId: null, model: null, prompt: null, taskMetadata: { ...NON_COMMITTING_COORDINATOR_METADATA, readOnly: true } },
// do-replan audits PLAN.md after open PRs and stale branches have been cleaned up,
// so the plan reflects what actually merged.
'do-replan': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, runAfter: ['pr-reviewer', 'branch-reconcile'], taskMetadata: { useWorktree: true, openPR: true } },
Expand Down
5 changes: 5 additions & 0 deletions server/services/taskScheduleStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ async function readSchedule() {
const defaultTask = DEFAULT_TASK_INTERVALS[taskType];
const loadedTask = loaded.tasks?.[taskType] || {};
const merged = { ...defaultTask, ...loadedTask };
// A shipped task's feature association is code-owned, like its identity.
// Do not let a stale persisted snapshot retain, replace, or remove the gate
// when a later PortOS version changes the registry.
if (defaultTask.feature) merged.feature = defaultTask.feature;
else delete merged.feature;
// Deep-merge taskMetadata: preserve explicit null (clears metadata), otherwise merge defaults with stored
// Only spread if loadedTask.taskMetadata is a plain object to avoid corrupting config
if (defaultTask.taskMetadata && loadedTask.taskMetadata !== null) {
Expand Down