diff --git a/.changelog/NEXT.md b/.changelog/NEXT.md index 61259b24..8475b2fa 100644 --- a/.changelog/NEXT.md +++ b/.changelog/NEXT.md @@ -17,6 +17,8 @@ - Personal Goals dashboard widget: shows top-level goals with progress bars, category icons, horizon labels, and stall detection (14+ days idle) — replaces broken CoS task-based goal widget that never rendered ## Changed +- Exported `cleanupAgentWorktree` from `subAgentSpawner.js` with dedicated test file covering the openPR completion path (push, PR creation, failure preservation, auto-merge fallback) +- Exported `isTruthyMeta` from `subAgentSpawner.js` and `applyAppWorktreeDefault` from `cos.js`; tests now import production helpers instead of duplicating their logic inline - Consolidated 5 duplicate `timeAgo`/`formatTimeAgo` implementations into a single `timeAgo()` in `utils/formatters.js` - Deduplicated `stripCodeFences` from insightsService — now imports from shared `lib/aiProvider.js` - Added `MINUTE` time constant to `lib/fileUtils.js` alongside existing `HOUR`/`DAY` @@ -40,6 +42,7 @@ - Chart date range comparison uses local date strings instead of UTC Date objects — fixes today/yesterday missing from 7-day chart - Empty entry objects no longer accumulate in daily-log.json after moving the last item from a date - README screenshot labels corrected to match actual screenshot content (all 6 were mislabeled) +- Zod `.default(false)` on `archived`, `defaultUseWorktree`, and `defaultOpenPR` in `appSchema` caused `appUpdateSchema.partial()` to inject `false` for omitted fields, silently overwriting stored `true` values during partial updates ## Removed - Dead code: `euclideanDistance`, `averageVectors`, `similarityMatrix` from `vectorMath.js` (unused outside tests) diff --git a/client/src/components/apps/EditAppModal.jsx b/client/src/components/apps/EditAppModal.jsx index 035a9663..29d2e46a 100644 --- a/client/src/components/apps/EditAppModal.jsx +++ b/client/src/components/apps/EditAppModal.jsx @@ -1,6 +1,6 @@ import { useState, useEffect } from 'react'; import { Link } from 'react-router-dom'; -import { ChevronDown, ChevronUp, GitBranch } from 'lucide-react'; +import { ChevronDown, ChevronUp, GitBranch, GitPullRequest } from 'lucide-react'; import IconPicker from '../IconPicker'; import * as api from '../../services/api'; @@ -16,7 +16,8 @@ export default function EditAppModal({ app, onClose, onSave }) { startCommands: (app.startCommands || []).join('\n'), pm2ProcessNames: (app.pm2ProcessNames || []).join(', '), editorCommand: app.editorCommand || 'code .', - defaultUseWorktree: app.defaultUseWorktree || false, + defaultOpenPR: app.defaultOpenPR || false, + defaultUseWorktree: app.defaultUseWorktree || app.defaultOpenPR || false, jiraEnabled: app.jira?.enabled || false, jiraInstanceId: app.jira?.instanceId || '', jiraProjectKey: app.jira?.projectKey || '', @@ -90,7 +91,8 @@ export default function EditAppModal({ app, onClose, onSave }) { ? formData.pm2ProcessNames.split(',').map(s => s.trim()).filter(Boolean) : undefined, editorCommand: formData.editorCommand || undefined, - defaultUseWorktree: formData.defaultUseWorktree, + defaultUseWorktree: formData.defaultUseWorktree || formData.defaultOpenPR, + defaultOpenPR: formData.defaultOpenPR, jira: formData.jiraEnabled ? { enabled: true, instanceId: formData.jiraInstanceId || undefined, @@ -242,11 +244,26 @@ export default function EditAppModal({ app, onClose, onSave }) { setFormData({ ...formData, defaultUseWorktree: e.target.checked })} + onChange={e => { + const updates = { defaultUseWorktree: e.target.checked }; + if (!e.target.checked) updates.defaultOpenPR = false; + setFormData(prev => ({ ...prev, ...updates })); + }} className="rounded border-port-border bg-port-bg text-port-accent focus:ring-port-accent" /> - Default to Worktree + PR for new tasks + Default to Worktree for new tasks + + {/* JIRA Integration Section */} diff --git a/client/src/components/cos/TaskAddForm.jsx b/client/src/components/cos/TaskAddForm.jsx index cfdaf9b1..c86f60f5 100644 --- a/client/src/components/cos/TaskAddForm.jsx +++ b/client/src/components/cos/TaskAddForm.jsx @@ -1,5 +1,5 @@ import { useState, useEffect, useRef, useMemo, useCallback } from 'react'; -import { Plus, Image, X, ChevronDown, ChevronRight, Sparkles, Loader2, Paperclip, FileText, Zap, Bookmark, Ticket, GitBranch, Wand2, RefreshCw } from 'lucide-react'; +import { Plus, Image, X, ChevronDown, ChevronRight, Sparkles, Loader2, Paperclip, FileText, Zap, Bookmark, Ticket, GitBranch, GitPullRequest, Wand2, RefreshCw } from 'lucide-react'; import toast from 'react-hot-toast'; import * as api from '../../services/api'; import { processScreenshotUploads, processAttachmentUploads } from '../../utils/fileUpload'; @@ -11,6 +11,7 @@ export default function TaskAddForm({ providers, apps, onTaskAdded, compact = fa const [enhancePrompt, setEnhancePrompt] = useState(false); const [isEnhancing, setIsEnhancing] = useState(false); const [useWorktree, setUseWorktree] = useState(false); + const [openPR, setOpenPR] = useState(false); const [simplify, setSimplify] = useState(true); const [reviewLoop, setReviewLoop] = useState(false); const [createJiraTicket, setCreateJiraTicket] = useState(false); @@ -45,11 +46,15 @@ export default function TaskAddForm({ providers, apps, onTaskAdded, compact = fa ); const appHasJira = selectedApp?.jira?.enabled; - // Auto-toggle JIRA and worktree checkboxes when app selection changes + // Auto-toggle JIRA, worktree, and PR checkboxes when app selection changes useEffect(() => { const app = apps?.find(a => a.id === newTask.app); + const defaultOpenPR = !!app?.defaultOpenPR; + const defaultUseWorktree = !!app?.defaultUseWorktree || defaultOpenPR; setCreateJiraTicket(!!app?.jira?.enabled); - setUseWorktree(!!app?.defaultUseWorktree); + setUseWorktree(defaultUseWorktree); + setOpenPR(defaultOpenPR); + if (defaultOpenPR) setReviewLoop(false); }, [newTask.app, apps]); // Get models for selected provider @@ -182,10 +187,11 @@ export default function TaskAddForm({ providers, apps, onTaskAdded, compact = fa model: newTask.model || undefined, provider: newTask.provider || undefined, app: newTask.app || undefined, - createJiraTicket: createJiraTicket || undefined, - useWorktree: useWorktree || undefined, - simplify: simplify || undefined, - reviewLoop: reviewLoop || undefined, + createJiraTicket, + useWorktree, + openPR, + simplify, + reviewLoop, screenshots: screenshots.length > 0 ? screenshots.map(s => s.path) : undefined, attachments: attachments.length > 0 ? attachments.map(a => ({ filename: a.filename, @@ -211,6 +217,7 @@ export default function TaskAddForm({ providers, apps, onTaskAdded, compact = fa setEnhancePrompt(false); setCreateJiraTicket(false); setUseWorktree(false); + setOpenPR(false); setSimplify(true); setReviewLoop(false); setExpanded(false); @@ -377,12 +384,31 @@ export default function TaskAddForm({ providers, apps, onTaskAdded, compact = fa setUseWorktree(e.target.checked)} + onChange={(e) => { + setUseWorktree(e.target.checked); + if (!e.target.checked) setOpenPR(false); + }} className="w-4 h-4 rounded border-port-border bg-port-bg text-port-accent focus:ring-port-accent focus:ring-offset-0" /> - + - Worktree + PR + Worktree + + + diff --git a/client/src/components/cos/constants.js b/client/src/components/cos/constants.js index 1f2e89dd..69c1aa40 100644 --- a/client/src/components/cos/constants.js +++ b/client/src/components/cos/constants.js @@ -49,15 +49,18 @@ export const STATE_MESSAGES = { ideating: "Analyzing options...", }; -// Agent option toggles for task metadata (useWorktree, simplify, reviewLoop) +// Agent option toggles for task metadata (useWorktree, openPR, simplify, reviewLoop) export const AGENT_OPTIONS = [ - { field: 'useWorktree', label: 'Worktree + PR', shortLabel: 'WT', description: 'Work in an isolated git worktree on a feature branch, then open a PR. If unchecked, commits directly to the default branch.' }, + { field: 'useWorktree', label: 'Worktree', shortLabel: 'WT', description: 'Work in an isolated git worktree on a feature branch. If unchecked, commits directly to the default branch.' }, + { field: 'openPR', label: 'Open PR', shortLabel: 'PR', description: 'Open a pull request to the default branch (implies worktree). If unchecked with worktree enabled, auto-merges to the default branch on completion.' }, { field: 'simplify', label: 'Run /simplify', shortLabel: '/s', description: 'Review code for reuse and quality before committing' }, - { field: 'reviewLoop', label: 'Review Loop', shortLabel: 'RL', description: 'After opening a PR, run review feedback loop until checks pass and PR is approved' } + { field: 'reviewLoop', label: 'Review Loop', shortLabel: 'RL', description: 'After the agent opens a PR during its run, keep iterating on review feedback until checks pass. Only applies when Open PR is not enabled (manual PR creation by agent).' } ]; // Compute new taskMetadata after toggling a field in a per-app override. // Returns null when all overrides are cleared (inherit everything). +// Enforces invariant: openPR implies useWorktree (turning on openPR forces +// useWorktree on; turning off useWorktree forces openPR off). export function toggleAppMetadataOverride(overrideMetadata, globalMetadata, field) { const current = overrideMetadata || {}; const newMeta = { ...current }; @@ -67,6 +70,24 @@ export function toggleAppMetadataOverride(overrideMetadata, globalMetadata, fiel const effective = overrideMetadata?.[field] ?? globalMetadata?.[field] ?? false; newMeta[field] = !effective; } + + const resolve = (f) => newMeta[f] ?? globalMetadata?.[f] ?? false; + + // Apply useWorktree-off rule first (based on the toggled field's new value) + if (field === 'useWorktree' && !newMeta.useWorktree && newMeta.useWorktree !== undefined) { + // User explicitly toggled worktree off — force openPR off + newMeta.openPR = false; + } else if (field === 'openPR' && resolve('openPR') && !resolve('useWorktree')) { + // User toggled openPR on — force useWorktree on + newMeta.useWorktree = true; + } + + // Clean entries that match the global value (revert to inherit) + for (const key of Object.keys(newMeta)) { + if (newMeta[key] === (globalMetadata?.[key] ?? false)) { + delete newMeta[key]; + } + } return Object.keys(newMeta).length ? newMeta : null; } diff --git a/client/src/components/cos/tabs/ScheduleTab.jsx b/client/src/components/cos/tabs/ScheduleTab.jsx index 203a3290..cc301c76 100644 --- a/client/src/components/cos/tabs/ScheduleTab.jsx +++ b/client/src/components/cos/tabs/ScheduleTab.jsx @@ -23,10 +23,22 @@ const INTERVAL_DESCRIPTIONS = { custom: 'Custom interval' }; +// Toggle a global taskMetadata field, enforcing the openPR→useWorktree invariant. function toggleMetadataField(metadata, field) { const current = metadata || {}; const newMeta = { ...current, [field]: !current[field] }; - const active = Object.fromEntries(Object.entries(newMeta).filter(([, v]) => v)); + // openPR requires useWorktree + if (newMeta.openPR && !newMeta.useWorktree) { + newMeta.useWorktree = true; + } + // useWorktree off means openPR must be off + if (newMeta.useWorktree === false && newMeta.openPR) { + newMeta.openPR = false; + } + // This is the GLOBAL taskMetadata config (the app-level default itself), not a per-task-type + // override. Only persist true values; absent keys default to disabled. Per-task-type overrides + // are handled separately by toggleAppMetadataOverride which supports tri-state (inherit/true/false). + const active = Object.fromEntries(Object.entries(newMeta).filter(([, v]) => v === true)); return Object.keys(active).length ? active : null; } diff --git a/server/lib/taskParser.js b/server/lib/taskParser.js index 8bbd5377..b786daf0 100644 --- a/server/lib/taskParser.js +++ b/server/lib/taskParser.js @@ -137,14 +137,21 @@ function escapeNewlines(value) { /** * Parse metadata line (indented under task) - * Format: - Key: Value + * Format: - key: Value + * Keys are written in camelCase (e.g., openPR, useWorktree, reviewLoop). + * Legacy Title-Case keys (e.g., Context, App) are accepted and normalized + * to camelCase by lowercasing the first character. */ function parseMetadataLine(line) { const match = line.match(/^\s+-\s*(\w+):\s*(.+)$/); if (!match) return null; + // Normalize key: lowercase first character to handle legacy Title-Case keys (Context→context, + // App→app) while preserving camelCase keys (openPR stays openPR, useWorktree stays useWorktree) + const rawKey = match[1]; + const key = rawKey.charAt(0).toLowerCase() + rawKey.slice(1); return { - key: match[1].toLowerCase(), + key, value: unescapeNewlines(match[2].trim()) }; } @@ -255,9 +262,8 @@ export function generateTasksMarkdown(tasks, includeApprovalFlags = false) { // Add metadata (escape newlines in values for single-line storage) for (const [key, value] of Object.entries(task.metadata)) { - const capitalizedKey = key.charAt(0).toUpperCase() + key.slice(1); const escapedValue = escapeNewlines(String(value)); - lines.push(` - ${capitalizedKey}: ${escapedValue}`); + lines.push(` - ${key}: ${escapedValue}`); } } diff --git a/server/lib/taskParser.test.js b/server/lib/taskParser.test.js index d1093d37..8995e660 100644 --- a/server/lib/taskParser.test.js +++ b/server/lib/taskParser.test.js @@ -105,11 +105,29 @@ describe('Task Parser', () => { const tasks = parseTasksMarkdown(markdown); expect(tasks).toHaveLength(1); + // Legacy Title-Case keys are normalized to camelCase (Context→context) expect(tasks[0].metadata.context).toBe('User reported issue'); expect(tasks[0].metadata.app).toBe('my-app'); expect(tasks[0].metadata.model).toBe('claude-sonnet'); }); + it('should preserve camelCase metadata keys', () => { + const markdown = `# Tasks + +## Pending +- [ ] #task-001 | HIGH | Fix the bug + - openPR: true + - useWorktree: true + - reviewLoop: false`; + + const tasks = parseTasksMarkdown(markdown); + + expect(tasks).toHaveLength(1); + expect(tasks[0].metadata.openPR).toBe('true'); + expect(tasks[0].metadata.useWorktree).toBe('true'); + expect(tasks[0].metadata.reviewLoop).toBe('false'); + }); + it('should handle empty content', () => { const tasks = parseTasksMarkdown(''); expect(tasks).toHaveLength(0); @@ -261,8 +279,8 @@ describe('Task Parser', () => { const markdown = generateTasksMarkdown(tasks); - expect(markdown).toContain('- Context: Some context'); - expect(markdown).toContain('- App: my-app'); + expect(markdown).toContain('- context: Some context'); + expect(markdown).toContain('- app: my-app'); }); it('should escape newlines in metadata values for round-trip preservation', () => { diff --git a/server/lib/validation.js b/server/lib/validation.js index bec21dad..c8bb8240 100644 --- a/server/lib/validation.js +++ b/server/lib/validation.js @@ -190,14 +190,15 @@ export const appSchema = z.object({ appIconPath: z.string().nullable().optional(), // Absolute path to detected app icon image editorCommand: z.string().optional(), description: z.string().optional(), - archived: z.boolean().optional().default(false), + archived: z.boolean().optional(), pm2Home: z.string().optional(), // Custom PM2_HOME path for apps that run in their own PM2 instance disabledTaskTypes: z.array(z.string()).optional(), // Legacy: migrated to taskTypeOverrides taskTypeOverrides: z.record(z.object({ enabled: z.boolean().optional(), interval: z.string().nullable().optional() })).optional(), // Per-task overrides: { [taskType]: { enabled, interval } } - defaultUseWorktree: z.boolean().optional().default(false), + defaultUseWorktree: z.boolean().optional(), + defaultOpenPR: z.boolean().optional(), jira: jiraConfigSchema.optional().nullable(), datadog: datadogConfigSchema.optional().nullable() }); @@ -542,7 +543,7 @@ export function validateRequest(schema, data) { // TASK METADATA SANITIZATION // ============================================================================= -const ALLOWED_TASK_METADATA_KEYS = ['useWorktree', 'simplify', 'reviewLoop']; +const ALLOWED_TASK_METADATA_KEYS = ['useWorktree', 'openPR', 'simplify', 'reviewLoop']; /** * Sanitize taskMetadata to only allowed agent-option keys with boolean values. diff --git a/server/lib/validation.test.js b/server/lib/validation.test.js index 9643d8da..b402c983 100644 --- a/server/lib/validation.test.js +++ b/server/lib/validation.test.js @@ -191,6 +191,15 @@ describe('validation.js', () => { const result = appUpdateSchema.safeParse(update); expect(result.success).toBe(false); }); + + it('should not inject default values for omitted boolean fields', () => { + const update = { name: 'Updated Name' }; + const result = appUpdateSchema.safeParse(update); + expect(result.success).toBe(true); + expect(result.data).not.toHaveProperty('archived'); + expect(result.data).not.toHaveProperty('defaultUseWorktree'); + expect(result.data).not.toHaveProperty('defaultOpenPR'); + }); }); describe('providerSchema', () => { @@ -453,6 +462,14 @@ describe('validation.js', () => { expect(sanitizeTaskMetadata({ useWorktree: true, simplify: false })).toEqual({ useWorktree: true, simplify: false }); }); + it('should accept openPR as an allowed metadata key', () => { + expect(sanitizeTaskMetadata({ openPR: true })).toEqual({ openPR: true }); + expect(sanitizeTaskMetadata({ openPR: false })).toEqual({ openPR: false }); + expect(sanitizeTaskMetadata({ useWorktree: true, openPR: true })).toEqual({ useWorktree: true, openPR: true }); + expect(sanitizeTaskMetadata({ useWorktree: true, openPR: true, simplify: true, reviewLoop: false })) + .toEqual({ useWorktree: true, openPR: true, simplify: true, reviewLoop: false }); + }); + it('should drop non-boolean values for allowed keys', () => { expect(sanitizeTaskMetadata({ useWorktree: 'yes' })).toBeNull(); expect(sanitizeTaskMetadata({ simplify: 1 })).toBeNull(); diff --git a/server/routes/cos.js b/server/routes/cos.js index dd5c0936..d6658264 100644 --- a/server/routes/cos.js +++ b/server/routes/cos.js @@ -191,13 +191,15 @@ router.post('/tasks/enhance', asyncHandler(async (req, res) => { // POST /api/cos/tasks - Add a new task router.post('/tasks', asyncHandler(async (req, res) => { - const { description, priority, context, model, provider, app, type = 'user', approvalRequired, screenshots, attachments, position = 'bottom', createJiraTicket, jiraTicketId, jiraTicketUrl, useWorktree, simplify, reviewLoop } = req.body; + const { description, priority, context, model, provider, app, type = 'user', approvalRequired, screenshots, attachments, position = 'bottom', createJiraTicket, jiraTicketId, jiraTicketUrl, useWorktree, openPR, simplify, reviewLoop } = req.body; if (!description) { throw new ServerError('Description is required', { status: 400, code: 'VALIDATION_ERROR' }); } - const taskData = { description, priority, context, model, provider, app, approvalRequired, screenshots, attachments, position, createJiraTicket, jiraTicketId, jiraTicketUrl, useWorktree, simplify, reviewLoop }; + // Coerce boolean flags — values from req.body may arrive as strings like 'false' (truthy in JS) + const toBool = (v) => v === true || v === 'true' ? true : v === false || v === 'false' ? false : undefined; + const taskData = { description, priority, context, model, provider, app, approvalRequired, screenshots, attachments, position, createJiraTicket: toBool(createJiraTicket), jiraTicketId, jiraTicketUrl, useWorktree: toBool(useWorktree), openPR: toBool(openPR), simplify: toBool(simplify), reviewLoop: toBool(reviewLoop) }; const result = await cos.addTask(taskData, type); if (result?.duplicate) { diff --git a/server/services/cleanupAgentWorktree.test.js b/server/services/cleanupAgentWorktree.test.js new file mode 100644 index 00000000..89a061bc --- /dev/null +++ b/server/services/cleanupAgentWorktree.test.js @@ -0,0 +1,370 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// --- Mock every dependency subAgentSpawner.js imports --- + +vi.mock('child_process', () => ({ + spawn: vi.fn(), + execSync: vi.fn() +})); + +vi.mock('fs/promises', () => ({ + writeFile: vi.fn().mockResolvedValue(undefined), + mkdir: vi.fn().mockResolvedValue(undefined), + readFile: vi.fn().mockResolvedValue(''), + readdir: vi.fn().mockResolvedValue([]), + rm: vi.fn().mockResolvedValue(undefined), + stat: vi.fn().mockResolvedValue({ isDirectory: () => true }) +})); + +vi.mock('fs', () => ({ + existsSync: vi.fn(() => false) +})); + +vi.mock('uuid', () => ({ + v4: vi.fn(() => 'mock-uuid') +})); + +vi.mock('./cos.js', () => ({ + cosEvents: { on: vi.fn(), emit: vi.fn() }, + registerAgent: vi.fn().mockResolvedValue(undefined), + updateAgent: vi.fn().mockResolvedValue(undefined), + completeAgent: vi.fn().mockResolvedValue(undefined), + appendAgentOutput: vi.fn().mockResolvedValue(undefined), + getConfig: vi.fn().mockResolvedValue({}), + updateTask: vi.fn().mockResolvedValue(undefined), + addTask: vi.fn().mockResolvedValue(undefined), + emitLog: vi.fn(), + getTaskById: vi.fn().mockResolvedValue(null), + getAgent: vi.fn().mockResolvedValue(null) +})); + +vi.mock('./appActivity.js', () => ({ + startAppCooldown: vi.fn(), + markAppReviewCompleted: vi.fn() +})); + +vi.mock('./cosRunnerClient.js', () => ({ + isRunnerAvailable: vi.fn(() => false), + spawnAgentViaRunner: vi.fn(), + terminateAgentViaRunner: vi.fn(), + killAgentViaRunner: vi.fn(), + getAgentStatsFromRunner: vi.fn(), + initCosRunnerConnection: vi.fn(), + onCosRunnerEvent: vi.fn(), + getActiveAgentsFromRunner: vi.fn(() => []), + getRunnerHealth: vi.fn() +})); + +vi.mock('./providers.js', () => ({ + getActiveProvider: vi.fn(), + getProviderById: vi.fn(), + getAllProviders: vi.fn(() => []) +})); + +vi.mock('./usage.js', () => ({ + recordSession: vi.fn(), + recordMessages: vi.fn() +})); + +vi.mock('./providerStatus.js', () => ({ + isProviderAvailable: vi.fn(() => true), + markProviderUsageLimit: vi.fn(), + markProviderRateLimited: vi.fn(), + getFallbackProvider: vi.fn(), + getProviderStatus: vi.fn(), + initProviderStatus: vi.fn() +})); + +vi.mock('./promptService.js', () => ({ + buildPrompt: vi.fn() +})); + +vi.mock('./agents.js', () => ({ + registerSpawnedAgent: vi.fn(), + unregisterSpawnedAgent: vi.fn() +})); + +vi.mock('./memoryRetriever.js', () => ({ + getMemorySection: vi.fn() +})); + +vi.mock('./memoryExtractor.js', () => ({ + extractAndStoreMemories: vi.fn() +})); + +vi.mock('./digital-twin.js', () => ({ + getDigitalTwinForPrompt: vi.fn() +})); + +vi.mock('./taskLearning.js', () => ({ + suggestModelTier: vi.fn() +})); + +vi.mock('../lib/fileUtils.js', () => ({ + ensureDir: vi.fn().mockResolvedValue(undefined), + readJSONFile: vi.fn().mockResolvedValue({}), + PATHS: { + root: '/mock/root', + cosAgents: '/mock/root/data/cos/agents', + runs: '/mock/root/data/runs', + worktrees: '/mock/root/data/cos/worktrees', + data: '/mock/root/data', + cos: '/mock/root/data/cos' + } +})); + +vi.mock('./apps.js', () => ({ + getAppById: vi.fn() +})); + +vi.mock('./toolStateMachine.js', () => ({ + createToolExecution: vi.fn(), + startExecution: vi.fn(), + updateExecution: vi.fn(), + completeExecution: vi.fn(), + errorExecution: vi.fn(), + getExecution: vi.fn(), + getStats: vi.fn() +})); + +vi.mock('./thinkingLevels.js', () => ({ + resolveThinkingLevel: vi.fn(), + getModelForLevel: vi.fn(), + isLocalPreferred: vi.fn(() => false) +})); + +vi.mock('./executionLanes.js', () => ({ + determineLane: vi.fn(), + acquire: vi.fn(), + release: vi.fn(), + hasCapacity: vi.fn(() => true), + waitForLane: vi.fn() +})); + +vi.mock('./taskConflict.js', () => ({ + detectConflicts: vi.fn(() => []) +})); + +vi.mock('./worktreeManager.js', () => ({ + createWorktree: vi.fn(), + removeWorktree: vi.fn().mockResolvedValue(undefined), + cleanupOrphanedWorktrees: vi.fn() +})); + +vi.mock('./jira.js', () => ({ + default: {} +})); + +vi.mock('./git.js', () => ({ + push: vi.fn(), + getRepoBranches: vi.fn(), + createPR: vi.fn() +})); + +vi.mock('./runner.js', () => ({ + executeApiRun: vi.fn(), + executeCliRun: vi.fn(), + createRun: vi.fn() +})); + +// --- Import the function under test and the mocked dependencies --- + +import { cleanupAgentWorktree } from './subAgentSpawner.js'; +import { getAgent } from './cos.js'; +import { removeWorktree } from './worktreeManager.js'; +import * as git from './git.js'; + +// Helper: build a mock agent state for worktree agents +function mockWorktreeAgent(overrides = {}) { + return { + metadata: { + isWorktree: true, + isPersistentWorktree: false, + sourceWorkspace: '/mock/workspace', + worktreeBranch: 'cos/task-abc123', + workspacePath: '/mock/root/data/cos/worktrees/agent-1', + ...overrides + } + }; +} + +describe('cleanupAgentWorktree - openPR path', () => { + beforeEach(() => { + vi.clearAllMocks(); + // Default: agent is a worktree agent with valid metadata + getAgent.mockResolvedValue(mockWorktreeAgent()); + git.getRepoBranches.mockResolvedValue({ baseBranch: 'main', devBranch: null }); + }); + + it('should run PR flow when openPR is true and success is true', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: true, url: 'https://github.com/test/repo/pull/1' }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: 'Test task' }); + + expect(git.push).toHaveBeenCalledWith('/mock/root/data/cos/worktrees/agent-1', 'cos/task-abc123'); + expect(git.createPR).toHaveBeenCalledWith('/mock/root/data/cos/worktrees/agent-1', { + title: 'Test task', + body: expect.stringContaining('Test task'), + base: 'main', + head: 'cos/task-abc123' + }); + expect(removeWorktree).toHaveBeenCalledWith('agent-1', '/mock/workspace', 'cos/task-abc123', { merge: false }); + }); + + it('should call removeWorktree with merge: false after successful push and PR', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: true, url: 'https://github.com/test/repo/pull/2' }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true }); + + expect(removeWorktree).toHaveBeenCalledTimes(1); + expect(removeWorktree).toHaveBeenCalledWith( + 'agent-1', + '/mock/workspace', + 'cos/task-abc123', + { merge: false } + ); + }); + + it('should preserve worktree when push fails (no removeWorktree call)', async () => { + git.push.mockRejectedValue(new Error('push rejected')); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: 'Test task' }); + + expect(git.push).toHaveBeenCalled(); + expect(git.createPR).not.toHaveBeenCalled(); + expect(removeWorktree).not.toHaveBeenCalled(); + }); + + it('should preserve worktree when createPR returns { success: false }', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: false, error: 'PR already exists' }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: 'Test task' }); + + expect(git.createPR).toHaveBeenCalled(); + expect(removeWorktree).not.toHaveBeenCalled(); + }); + + it('should use auto-merge path when openPR is false (success)', async () => { + await cleanupAgentWorktree('agent-1', true, { openPR: false }); + + expect(git.push).not.toHaveBeenCalled(); + expect(git.createPR).not.toHaveBeenCalled(); + expect(removeWorktree).toHaveBeenCalledWith('agent-1', '/mock/workspace', 'cos/task-abc123', { merge: true }); + }); + + it('should use auto-merge path when openPR is not provided (defaults to false)', async () => { + await cleanupAgentWorktree('agent-1', true); + + expect(git.push).not.toHaveBeenCalled(); + expect(git.createPR).not.toHaveBeenCalled(); + expect(removeWorktree).toHaveBeenCalledWith('agent-1', '/mock/workspace', 'cos/task-abc123', { merge: true }); + }); + + it('should skip PR flow when openPR is true but success is false', async () => { + await cleanupAgentWorktree('agent-1', false, { openPR: true }); + + expect(git.push).not.toHaveBeenCalled(); + expect(git.createPR).not.toHaveBeenCalled(); + // Falls through to auto-merge path with merge: false (failure cleanup) + expect(removeWorktree).toHaveBeenCalledWith('agent-1', '/mock/workspace', 'cos/task-abc123', { merge: false }); + }); + + it('should use baseBranch as PR base (not devBranch, since worktrees are created from baseBranch)', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: true, url: 'https://github.com/test/repo/pull/3' }); + git.getRepoBranches.mockResolvedValue({ baseBranch: 'main', devBranch: 'develop' }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: 'Test' }); + + expect(git.createPR).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + base: 'main' + })); + }); + + it('should fall back to "main" when getRepoBranches fails', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: true, url: 'https://github.com/test/repo/pull/4' }); + git.getRepoBranches.mockRejectedValue(new Error('not a git repo')); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: 'Test' }); + + expect(git.createPR).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + base: 'main' + })); + }); + + it('should preserve worktree when createPR throws', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockRejectedValue(new Error('network error')); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: 'Test' }); + + // PR creation failed — worktree preserved for manual intervention + expect(removeWorktree).not.toHaveBeenCalled(); + }); + + it('should truncate long descriptions to 100 chars for PR title', async () => { + const longDesc = 'A'.repeat(200); + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: true, url: 'https://github.com/test/repo/pull/5' }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true, description: longDesc }); + + expect(git.createPR).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + title: 'A'.repeat(100) + })); + }); + + it('should use default description when none provided', async () => { + git.push.mockResolvedValue(undefined); + git.createPR.mockResolvedValue({ success: true, url: 'https://github.com/test/repo/pull/6' }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true }); + + expect(git.createPR).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ + title: 'CoS automated task', + body: expect.stringContaining('CoS automated task') + })); + }); + + // --- Early-exit guard tests --- + + it('should no-op when agent is not a worktree agent', async () => { + getAgent.mockResolvedValue({ metadata: { isWorktree: false } }); + + await cleanupAgentWorktree('agent-1', true, { openPR: true }); + + expect(git.push).not.toHaveBeenCalled(); + expect(removeWorktree).not.toHaveBeenCalled(); + }); + + it('should no-op when agent state is null', async () => { + getAgent.mockResolvedValue(null); + + await cleanupAgentWorktree('agent-1', true, { openPR: true }); + + expect(git.push).not.toHaveBeenCalled(); + expect(removeWorktree).not.toHaveBeenCalled(); + }); + + it('should no-op for persistent worktree agents', async () => { + getAgent.mockResolvedValue(mockWorktreeAgent({ isPersistentWorktree: true })); + + await cleanupAgentWorktree('agent-1', true, { openPR: true }); + + expect(git.push).not.toHaveBeenCalled(); + expect(removeWorktree).not.toHaveBeenCalled(); + }); + + it('should no-op when sourceWorkspace or worktreeBranch is missing', async () => { + getAgent.mockResolvedValue(mockWorktreeAgent({ sourceWorkspace: null })); + + await cleanupAgentWorktree('agent-1', true, { openPR: true }); + + expect(git.push).not.toHaveBeenCalled(); + expect(removeWorktree).not.toHaveBeenCalled(); + }); +}); diff --git a/server/services/cos.js b/server/services/cos.js index 30b17331..e11d8b1b 100644 --- a/server/services/cos.js +++ b/server/services/cos.js @@ -1866,12 +1866,43 @@ Use model: claude-opus-4-5-20251101 for thorough security analysis` * @param {Object} state - Current CoS state * @returns {Object} Generated task */ -// When an app explicitly sets defaultUseWorktree, override per-task-type metadata -function applyAppWorktreeDefault(metadata, app) { - if (app.defaultUseWorktree === true) { +// Apply app-level worktree/PR defaults only when not already set by task-type metadata. +// openPR is applied first since it implies useWorktree — this prevents defaultUseWorktree: false +// from blocking defaultOpenPR: true when both are app-level defaults. +export function applyAppWorktreeDefault(metadata, app) { + const taskTypeDisabledWorktree = metadata.useWorktree === false || metadata.useWorktree === 'false'; + + // Apply defaultOpenPR first (since openPR implies useWorktree) + if (metadata.openPR === undefined) { + if (app.defaultOpenPR === true && !taskTypeDisabledWorktree) { + metadata.openPR = true; + metadata.useWorktree = true; // openPR implies useWorktree + } else if (app.defaultOpenPR === false || taskTypeDisabledWorktree) { + metadata.openPR = false; + } + } + + // Apply defaultUseWorktree (only if not already set by task-type or openPR above) + if (metadata.useWorktree === undefined) { + // openPR implies useWorktree — don't let app default override explicit openPR: true + const explicitOpenPR = metadata.openPR === true || metadata.openPR === 'true'; + if (explicitOpenPR) { + metadata.useWorktree = true; + } else if (app.defaultUseWorktree === true) { + metadata.useWorktree = true; + } else if (app.defaultUseWorktree === false) { + metadata.useWorktree = false; + } + } + + // Final invariant: openPR implies useWorktree (normalize in both directions) + const finalOpenPR = metadata.openPR === true || metadata.openPR === 'true'; + const finalWorktreeOff = metadata.useWorktree === false || metadata.useWorktree === 'false'; + if (finalOpenPR && finalWorktreeOff) { + // openPR wins — force useWorktree on metadata.useWorktree = true; - } else if (app.defaultUseWorktree === false) { - metadata.useWorktree = false; + } else if (finalWorktreeOff) { + metadata.openPR = false; } } @@ -3190,9 +3221,17 @@ export async function addTask(taskData, taskType = 'user', { raw = false } = {}) if (taskData.provider) metadata.provider = taskData.provider; if (taskData.app) metadata.app = taskData.app; if (taskData.createJiraTicket) metadata.createJiraTicket = true; - if (taskData.useWorktree) metadata.useWorktree = true; - if (taskData.simplify) metadata.simplify = true; - if (taskData.reviewLoop) metadata.reviewLoop = true; + // Boolean flags: persist both true and false so users can explicitly override defaults. + // The string round-trip ('false' from TASKS.md) is handled by isTruthyMeta/isFalsyMeta. + // undefined means "use app defaults". + if (taskData.useWorktree === true) metadata.useWorktree = true; + else if (taskData.useWorktree === false) metadata.useWorktree = false; + if (taskData.openPR === true) metadata.openPR = true; + else if (taskData.openPR === false) metadata.openPR = false; + if (taskData.simplify === true) metadata.simplify = true; + else if (taskData.simplify === false) metadata.simplify = false; + if (taskData.reviewLoop === true) metadata.reviewLoop = true; + else if (taskData.reviewLoop === false) metadata.reviewLoop = false; if (taskData.jiraTicketId) metadata.jiraTicketId = taskData.jiraTicketId; if (taskData.jiraTicketUrl) metadata.jiraTicketUrl = taskData.jiraTicketUrl; if (taskData.screenshots?.length > 0) metadata.screenshots = taskData.screenshots; diff --git a/server/services/subAgentSpawner.js b/server/services/subAgentSpawner.js index 560b9116..f303a99d 100644 --- a/server/services/subAgentSpawner.js +++ b/server/services/subAgentSpawner.js @@ -39,6 +39,10 @@ const ROOT_DIR = PATHS.root; const AGENTS_DIR = PATHS.cosAgents; const RUNS_DIR = PATHS.runs; +// Metadata booleans may arrive as true/'true' or false/'false' (from JSON vs TASKS.md string round-trip) +export const isTruthyMeta = (value) => value === true || value === 'true'; +export const isFalsyMeta = (value) => value === false || value === 'false'; + /** * Extract task type key for learning lookup * Matches the format used in taskLearning.js for consistency @@ -1470,12 +1474,13 @@ export async function spawnAgentForTask(task) { } } - // Determine worktree usage: explicit user flag takes priority, then conflict-based auto-detection. - // The useWorktree metadata flag is set from the task creation UI checkbox. - // When true, always create a worktree (branch + PR). When not set, only create a - // worktree if conflict is detected with other running agents. + // Determine worktree usage: explicit user flags take priority, then conflict-based auto-detection. + // useWorktree: work in an isolated worktree branch + // openPR: open a PR to default branch (implies useWorktree) + // When neither is set, only create a worktree if conflict is detected with other running agents. let worktreeInfo = null; - const explicitWorktree = task.metadata?.useWorktree === 'true' || task.metadata?.useWorktree === true; + const explicitOpenPR = isTruthyMeta(task.metadata?.openPR); + const explicitWorktree = isTruthyMeta(task.metadata?.useWorktree) || explicitOpenPR; // Feature agent tasks: use persistent worktree instead of creating a new one if (task.metadata?.featureAgentRun && task.metadata?.featureAgentId) { @@ -1525,7 +1530,7 @@ export async function spawnAgentForTask(task) { agentId, worktreePath: worktreeInfo.worktreePath, branchName: worktreeInfo.branchName, baseBranch: worktreeInfo.baseBranch }); } - } else if (!jiraBranchName && task.metadata?.useWorktree !== false) { + } else if (!jiraBranchName && !isFalsyMeta(task.metadata?.useWorktree)) { // No explicit worktree requested and not explicitly disabled: use worktree only when conflict is detected const { getAgents } = await import('./cos.js'); const allAgents = await getAgents(); @@ -1939,7 +1944,8 @@ async function handleAgentCompletion(agentId, exitCode, success, duration) { // Clean up worktree if agent was using one (skip merge when JIRA branch — PR handles merge) if (!jiraBranch) { - await cleanupAgentWorktree(agentId, success); + const taskOpenPR = isTruthyMeta(agent.task?.metadata?.openPR); + await cleanupAgentWorktree(agentId, success, { openPR: taskOpenPR, description: task?.description }); } runnerAgents.delete(agentId); @@ -1948,9 +1954,10 @@ async function handleAgentCompletion(agentId, exitCode, success, duration) { /** * Clean up a worktree for a completed agent. * Reads worktree metadata from the agent's registered state and removes the worktree. - * On success, merges the worktree branch back to the source branch. + * When openPR is true, pushes the branch and creates a PR instead of auto-merging. + * Otherwise, merges the worktree branch back to the source branch on success. */ -async function cleanupAgentWorktree(agentId, success) { +export async function cleanupAgentWorktree(agentId, success, { openPR = false, description = null } = {}) { const { getAgent: getAgentState } = await import('./cos.js'); const agentState = await getAgentState(agentId).catch(() => null); if (!agentState?.metadata?.isWorktree) return; @@ -1960,6 +1967,60 @@ async function cleanupAgentWorktree(agentId, success) { const { sourceWorkspace, worktreeBranch } = agentState.metadata; if (!sourceWorkspace || !worktreeBranch) return; + // When openPR is set and task succeeded, push branch and create PR instead of auto-merging + if (openPR && success) { + emitLog('info', `🌳 Opening PR for worktree agent ${agentId} branch ${worktreeBranch}`, { agentId, branchName: worktreeBranch }); + + const worktreePath = agentState.metadata.workspacePath || join(PATHS.worktrees, agentId); + + // Push branch and resolve target branch in parallel + const [pushResult, branchInfo] = await Promise.all([ + git.push(worktreePath, worktreeBranch).then(() => true).catch(err => { + emitLog('warn', `🌳 Failed to push worktree branch ${worktreeBranch}: ${err.message}`, { agentId }); + return false; + }), + git.getRepoBranches(sourceWorkspace).catch(() => ({ baseBranch: null, devBranch: null })) + ]); + + // Only create PR if push succeeded; preserve worktree/branch for manual intervention if push fails + if (pushResult) { + // Use baseBranch (not devBranch) since worktrees are created from the default branch + const targetBranch = branchInfo.baseBranch || 'main'; + const taskDesc = description || 'CoS automated task'; + const prTitle = taskDesc.replace(/[\r\n]+/g, ' ').trim().substring(0, 100); + + const prResult = await git.createPR(worktreePath, { + title: prTitle, + body: `Automated PR created by PortOS Chief of Staff.\n\n**Task:** ${taskDesc}`, + base: targetBranch, + head: worktreeBranch + }).catch(err => { + emitLog('warn', `🌳 Failed to create PR for ${worktreeBranch}: ${err.message}`, { agentId }); + return null; + }); + + if (!prResult?.success) { + const reason = prResult?.error || 'unknown error (createPR returned null or threw)'; + emitLog('error', `🌳 PR creation failed for ${worktreeBranch}: ${reason}`, { agentId, branchName: worktreeBranch }); + // Preserve worktree/branch for manual PR creation + return; + } + + emitLog('success', `🌳 Created PR: ${prResult.url}`, { agentId, branchName: worktreeBranch }); + + // Remove worktree without merging (PR handles merge) + await removeWorktree(agentId, sourceWorkspace, worktreeBranch, { merge: false }).catch(err => { + emitLog('warn', `🌳 Worktree cleanup failed for ${agentId}: ${err.message}`, { agentId }); + }); + return; + } + + // Push failed — preserve worktree/branch for manual intervention (do NOT auto-merge in openPR mode) + emitLog('warn', `🌳 Push failed for ${worktreeBranch} — worktree preserved at ${worktreePath} for manual retry`, { agentId, branchName: worktreeBranch }); + return; + } + + // Default: auto-merge on success, just cleanup on failure emitLog('info', `🌳 Cleaning up worktree for agent ${agentId} (merge: ${success})`, { agentId, branchName: worktreeBranch, merge: success }); @@ -2187,7 +2248,10 @@ async function spawnDirectly(agentId, task, prompt, workspacePath, model, provid await processAgentCompletion(agentId, task, success, outputBuffer); // Clean up worktree if agent was using one - await cleanupAgentWorktree(agentId, success); + await cleanupAgentWorktree(agentId, success, { + openPR: isTruthyMeta(task.metadata?.openPR), + description: task.description + }); unregisterSpawnedAgent(agentData?.pid || claudeProcess.pid); activeAgents.delete(agentId); @@ -2620,6 +2684,7 @@ async function buildAgentPrompt(task, config, workspaceDir, worktreeInfo = null) const compactionSection = task.metadata?.compaction?.needed ? buildCompactionSection(task) : ''; // Build worktree context section if applicable + const willOpenPR = isTruthyMeta(task.metadata?.openPR); const worktreeSection = worktreeInfo ? ` ## Git Worktree Context You are working in an **isolated git worktree** to avoid conflicts with other agents working concurrently. @@ -2627,17 +2692,17 @@ You are working in an **isolated git worktree** to avoid conflicts with other ag - **Worktree Path**: \`${worktreeInfo.worktreePath}\` ${worktreeInfo.baseBranch ? `- **Based on**: \`${worktreeInfo.baseBranch}\` (latest from origin)` : ''} -**Important**: Commit your changes to this branch. Your commits will be automatically merged back to the main development branch when your task completes. Do NOT manually switch branches or modify the worktree configuration. +**Important**: Commit your changes to this branch.${willOpenPR ? ' Your commits will be submitted as a pull request to the default branch when your task completes.' : ' Your commits will be automatically merged back to the main development branch when your task completes.'} Do NOT manually switch branches or modify the worktree configuration. ` : ''; // Build simplify section if enabled - const simplifySection = task.metadata?.simplify ? ` + const simplifySection = isTruthyMeta(task.metadata?.simplify) ? ` ## Simplify Step After completing your work and before committing, run \`/simplify\` to review the changed code for reuse, quality, and efficiency. Fix any issues found before committing. ` : ''; - // Build review loop section if enabled - const reviewLoopSection = task.metadata?.reviewLoop ? ` + // Build review loop section if enabled (only when the agent creates the PR itself, not when openPR auto-creates it post-exit) + const reviewLoopSection = (isTruthyMeta(task.metadata?.reviewLoop) && !willOpenPR) ? ` ## Review Loop After opening the PR, run \`/do:rpr\` to resolve PR review feedback and complete the merge validation. Continue running the review loop until all checks pass and the PR is approved. ` : ''; @@ -2739,7 +2804,7 @@ ${skillSection ? `## Task-Type Skill Guidelines\n\n${skillSection}\n` : ''}${pla - Do not make unrelated changes - If blocked, explain clearly why - Never update the PortOS changelog (\`.changelog/\`) for work on managed apps — the PortOS changelog tracks PortOS core changes only -${task.metadata?.app && worktreeInfo ? `- **When done, create a pull request to the repo's default branch** (main/master) instead of committing directly to dev. Use the /pr skill or gh CLI to open the PR.` : `- Commit code after each feature or bug fix using the git tools or /cam skill`} +${task.metadata?.app && worktreeInfo && willOpenPR ? `- Commit code after each feature or bug fix using the git tools or /cam skill. A pull request will be automatically created when your task completes — do NOT open a PR manually.` : task.metadata?.app && worktreeInfo ? `- Commit code after each feature or bug fix using the git tools or /cam skill. Your worktree branch will be automatically merged back to the source branch when your task completes — do NOT open a PR.` : `- Commit code after each feature or bug fix using the git tools or /cam skill`} ## Git Hygiene (CRITICAL) - **Before starting work**, run \`git status\` to verify a clean working tree. If there are uncommitted changes from a previous agent or manual work, **stash or discard them** before proceeding — do NOT commit someone else's changes. diff --git a/server/services/subAgentSpawner.test.js b/server/services/subAgentSpawner.test.js index ecf6d9d3..5c5d2941 100644 --- a/server/services/subAgentSpawner.test.js +++ b/server/services/subAgentSpawner.test.js @@ -1,11 +1,13 @@ import { describe, it, expect } from 'vitest'; +import { isTruthyMeta, isFalsyMeta } from './subAgentSpawner.js'; +import { applyAppWorktreeDefault } from './cos.js'; /** * Tests for the subAgentSpawner service * - * Note: We test the pure functions directly by extracting their logic. - * The spawner has complex dependencies (process spawning, file system, etc.) - * so we focus on the decision-making logic that can be unit tested. + * Note: We test the pure functions directly by importing them from production. + * For functions with complex dependencies (process spawning, file system, etc.) + * we focus on the decision-making logic that can be unit tested. */ // Test model selection logic @@ -671,4 +673,147 @@ describe('Task Failure Retry Logic', () => { }); }); + // --- isTruthyMeta helper (imported from production) --- + describe('isTruthyMeta', () => { + it('should return true for boolean true', () => { + expect(isTruthyMeta(true)).toBe(true); + }); + + it('should return true for string "true"', () => { + expect(isTruthyMeta('true')).toBe(true); + }); + + it('should return false for false', () => { + expect(isTruthyMeta(false)).toBe(false); + }); + + it('should return false for undefined/null/other values', () => { + expect(isTruthyMeta(undefined)).toBe(false); + expect(isTruthyMeta(null)).toBe(false); + expect(isTruthyMeta('false')).toBe(false); + expect(isTruthyMeta(1)).toBe(false); + expect(isTruthyMeta('')).toBe(false); + }); + }); + + // --- isFalsyMeta helper (imported from production) --- + describe('isFalsyMeta', () => { + it('should return true for boolean false', () => { + expect(isFalsyMeta(false)).toBe(true); + }); + + it('should return true for string "false"', () => { + expect(isFalsyMeta('false')).toBe(true); + }); + + it('should return false for true', () => { + expect(isFalsyMeta(true)).toBe(false); + }); + + it('should return false for undefined/null/other values', () => { + expect(isFalsyMeta(undefined)).toBe(false); + expect(isFalsyMeta(null)).toBe(false); + expect(isFalsyMeta('true')).toBe(false); + expect(isFalsyMeta(0)).toBe(false); + expect(isFalsyMeta('')).toBe(false); + }); + }); + + // --- openPR worktree decision logic (uses imported isTruthyMeta) --- + describe('openPR/useWorktree decision logic', () => { + function resolveWorktreeFlags(metadata) { + const explicitOpenPR = isTruthyMeta(metadata?.openPR); + const explicitWorktree = isTruthyMeta(metadata?.useWorktree) || explicitOpenPR; + return { explicitOpenPR, explicitWorktree }; + } + + it('should not use worktree or PR when neither is set', () => { + const result = resolveWorktreeFlags({}); + expect(result.explicitWorktree).toBe(false); + expect(result.explicitOpenPR).toBe(false); + }); + + it('should use worktree but not PR when only useWorktree is set', () => { + const result = resolveWorktreeFlags({ useWorktree: true }); + expect(result.explicitWorktree).toBe(true); + expect(result.explicitOpenPR).toBe(false); + }); + + it('should imply worktree when openPR is set', () => { + const result = resolveWorktreeFlags({ openPR: true }); + expect(result.explicitWorktree).toBe(true); + expect(result.explicitOpenPR).toBe(true); + }); + + it('should use both when both are set', () => { + const result = resolveWorktreeFlags({ useWorktree: true, openPR: true }); + expect(result.explicitWorktree).toBe(true); + expect(result.explicitOpenPR).toBe(true); + }); + + it('should handle string "true" values from form/URL params', () => { + const result = resolveWorktreeFlags({ useWorktree: 'true', openPR: 'true' }); + expect(result.explicitWorktree).toBe(true); + expect(result.explicitOpenPR).toBe(true); + }); + + it('should not use worktree when useWorktree is false and openPR is false', () => { + const result = resolveWorktreeFlags({ useWorktree: false, openPR: false }); + expect(result.explicitWorktree).toBe(false); + expect(result.explicitOpenPR).toBe(false); + }); + }); + + // --- applyAppWorktreeDefault logic (imported from production cos.js) --- + describe('applyAppWorktreeDefault', () => { + it('should fill defaults when metadata has no worktree/openPR fields', () => { + const metadata = {}; + applyAppWorktreeDefault(metadata, { defaultUseWorktree: true, defaultOpenPR: true }); + expect(metadata.useWorktree).toBe(true); + expect(metadata.openPR).toBe(true); + }); + + it('should not override task-type metadata that is already set', () => { + const metadata = { useWorktree: false, openPR: false }; + applyAppWorktreeDefault(metadata, { defaultUseWorktree: true, defaultOpenPR: true }); + expect(metadata.useWorktree).toBe(false); + expect(metadata.openPR).toBe(false); + }); + + it('should enforce openPR=false when useWorktree=false', () => { + const metadata = { useWorktree: false }; + applyAppWorktreeDefault(metadata, { defaultOpenPR: true }); + expect(metadata.openPR).toBe(false); + }); + + it('should imply useWorktree when defaultOpenPR is true', () => { + const metadata = {}; + applyAppWorktreeDefault(metadata, { defaultOpenPR: true }); + expect(metadata.useWorktree).toBe(true); + expect(metadata.openPR).toBe(true); + }); + + it('should not let defaultUseWorktree:false override explicit openPR:true', () => { + const metadata = { openPR: true }; + applyAppWorktreeDefault(metadata, { defaultUseWorktree: false }); + // openPR implies useWorktree — app default must not override explicit openPR + expect(metadata.useWorktree).toBe(true); + expect(metadata.openPR).toBe(true); + }); + + it('should handle openPR:"true" string the same as boolean true', () => { + const metadata = { openPR: 'true' }; + applyAppWorktreeDefault(metadata, { defaultUseWorktree: false }); + expect(metadata.useWorktree).toBe(true); + expect(metadata.openPR).toBe('true'); + }); + + it('should leave metadata unchanged when app has no defaults', () => { + const metadata = { useWorktree: true, openPR: true }; + applyAppWorktreeDefault(metadata, {}); + expect(metadata.useWorktree).toBe(true); + expect(metadata.openPR).toBe(true); + }); + }); + }); diff --git a/server/services/taskSchedule.js b/server/services/taskSchedule.js index dc958cbb..3c9c5b8c 100644 --- a/server/services/taskSchedule.js +++ b/server/services/taskSchedule.js @@ -872,12 +872,12 @@ const DEFAULT_TASK_INTERVALS = { 'documentation': { type: INTERVAL_TYPES.ONCE, enabled: false, providerId: null, model: null, prompt: null }, 'ui-bugs': { type: INTERVAL_TYPES.ON_DEMAND, enabled: false, providerId: null, model: null, prompt: null }, 'mobile-responsive': { type: INTERVAL_TYPES.ON_DEMAND, enabled: false, providerId: null, model: null, prompt: null }, - 'feature-ideas': { type: INTERVAL_TYPES.DAILY, enabled: false, providerId: null, model: null, prompt: null, taskMetadata: { useWorktree: true, simplify: true } }, + 'feature-ideas': { type: INTERVAL_TYPES.DAILY, enabled: false, providerId: null, model: null, prompt: null, taskMetadata: { useWorktree: true, openPR: true, simplify: true } }, 'error-handling': { type: INTERVAL_TYPES.ROTATION, enabled: false, providerId: null, model: null, prompt: null }, 'typing': { type: INTERVAL_TYPES.ONCE, enabled: false, providerId: null, model: null, prompt: null }, 'release-check': { type: INTERVAL_TYPES.ON_DEMAND, enabled: false, providerId: null, model: null, prompt: null }, 'pr-reviewer': { type: INTERVAL_TYPES.CUSTOM, intervalMs: 7200000, enabled: false, weekdaysOnly: true, providerId: null, model: null, prompt: null }, - 'jira-sprint-manager': { type: INTERVAL_TYPES.DAILY, enabled: false, weekdaysOnly: true, providerId: null, model: null, prompt: null, taskMetadata: { useWorktree: true, simplify: true } } + 'jira-sprint-manager': { type: INTERVAL_TYPES.DAILY, enabled: false, weekdaysOnly: true, providerId: null, model: null, prompt: null, taskMetadata: { useWorktree: true, openPR: true, simplify: true } } }; /**