feat(cos): split worktree and PR configs into independent settings#101
feat(cos): split worktree and PR configs into independent settings#101
Conversation
There was a problem hiding this comment.
Pull request overview
This PR splits the previously combined “Worktree + PR” behavior into two independent flags (useWorktree and openPR), adds an app-level default for PR creation, and wires the new flag through client UI, server validation, task creation, and agent completion cleanup behavior.
Changes:
- Introduces
openPRalongsideuseWorktree, includingdefaultOpenPRat the app configuration level. - Updates client forms/modals to display separate Worktree/Open PR checkboxes with dependency behavior.
- Updates agent spawning/cleanup to treat
openPRas implying worktree and to create PRs on successful completion.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/services/taskSchedule.js | Enables openPR: true in taskMetadata defaults for selected scheduled task types. |
| server/services/subAgentSpawner.js | Adds openPR handling in worktree decision logic, prompt text, and completion cleanup (push + PR creation). |
| server/services/cos.js | Extends app-default application logic to include defaultOpenPR and openPR cascading. |
| server/routes/cos.js | Passes openPR through the task creation route payload. |
| server/lib/validation.js | Adds defaultOpenPR to app schema and openPR to allowed task metadata keys. |
| client/src/components/cos/constants.js | Splits AGENT_OPTIONS entries into Worktree and Open PR. |
| client/src/components/cos/TaskAddForm.jsx | Adds separate Worktree/Open PR toggles and submits openPR in task creation. |
| client/src/components/apps/EditAppModal.jsx | Adds defaultOpenPR checkbox dependent on defaultUseWorktree. |
Comments suppressed due to low confidence (1)
server/services/subAgentSpawner.js:1963
- cleanupAgentWorktree now supports an openPR mode, but not all call sites pass that flag (e.g., the direct-spawn completion path still calls cleanupAgentWorktree(agentId, success) with no options). That means tasks with metadata.openPR=true can still be auto-merged instead of pushed/PR’d depending on how the agent was spawned. Please ensure all cleanupAgentWorktree invocations pass the task’s openPR (and description for PR title/body) so behavior is consistent across runner vs direct modes.
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;
// Skip cleanup for persistent feature agent worktrees (they survive across runs)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Split the combined "Worktree + PR" toggle into two separate configs: - useWorktree: work in an isolated git worktree on a feature branch - openPR: open a pull request to the default branch (implies worktree) Behavior matrix: - Neither: commit directly to default branch - Worktree only: work in worktree, auto-merge on completion - Open PR (implies worktree): work in worktree, push + create PR App-level defaults (defaultUseWorktree, defaultOpenPR) cascade into manual and scheduled task metadata with per-task override support.
- Fall back to auto-merge when push fails (prevents commit loss) - Fix conflicting prompt: system auto-creates PR, agent no longer told to - Coerce openPR+useWorktree state in TaskAddForm (openPR implies worktree) - applyAppWorktreeDefault only fills unset fields, respects task-type metadata - Add 17 unit tests for isTruthyMeta, worktree flag resolution, and app defaults
…thy bugs Boolean flags (useWorktree, openPR, simplify, reviewLoop) from request body can arrive as strings like 'false' which are truthy in JavaScript. Added explicit boolean coercion in the route layer via a toBool helper, and changed the service layer to use strict === checks that persist both true and false values in metadata (so app-level defaults don't override explicit user intent).
… in tests Export isTruthyMeta from subAgentSpawner.js and applyAppWorktreeDefault from cos.js so tests import the real functions rather than maintaining drift-prone inline copies. Addresses Copilot review PRRT_kwDOQx8jQ8515lKk.
… auto-merging When push fails in openPR mode, the caller opted for PR-based delivery. Falling back to auto-merge into the local default branch is unexpected. Instead, preserve the worktree and branch so the user can investigate and retry the push manually. Addresses review thread PRRT_kwDOQx8jQ8515lKs
reviewLoop requires a PR to exist during the agent's run, but openPR creates the PR after the agent exits. Disable reviewLoop checkbox when openPR is checked, gate review loop prompt section on !willOpenPR, and update description to clarify.
atomantic
force-pushed
the
cos/task-mn0ligpt/agent-4fd16660
branch
from
5e687d6 to
74b9b61
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
server/services/subAgentSpawner.js:2669
simplifyis checked via truthiness (task.metadata?.simplify), but task metadata values parsed from TASKS.md are strings. Ifsimplifyis ever persisted asfalse, it will be read as'false'(truthy) and this section will still be enabled. Use the same boolean coercion helper as worktree/PR flags (e.g.isTruthyMeta/toBoolMeta) for simplify/reviewLoop checks.
const digitalTwinSection = await getDigitalTwinForPrompt({
maxTokens: config.digitalTwin?.maxContextTokens || config.soul?.maxContextTokens || 2000
}).catch(err => {
console.log(`⚠️ Digital twin context retrieval failed: ${err.message}`);
return null;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Only persist true to TASKS.md metadata (not false) to avoid 'false' string being truthy when parsed back - Add isFalsyMeta helper for symmetric string coercion - Apply toBool to createJiraTicket in routes - Send boolean values directly from TaskAddForm (not || undefined) - Fix stale comment about auto-merge fallback
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…event partial updates overwriting stored values Zod's .default(false) on archived, defaultUseWorktree, and defaultOpenPR caused appUpdateSchema (appSchema.partial()) to inject false for omitted fields, silently overwriting stored true values during partial updates.
- Clear reviewLoop state when app with defaultOpenPR is selected - Respect explicit openPR:true in applyAppWorktreeDefault cascade - Handle createPR non-throwing failures in openPR cleanup path
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Enforce openPR→useWorktree invariant in toggleAppMetadataOverride and ScheduleTab - Normalize EditAppModal state so defaultOpenPR implies defaultUseWorktree - Add 16 unit tests for cleanupAgentWorktree openPR path - Export cleanupAgentWorktree for testability
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Preserve worktree when createPR throws (was incorrectly falling through) - Keep explicit false overrides in ScheduleTab metadata (filter !== undefined) - Handle string 'false' in applyAppWorktreeDefault invariant checks
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Strip newlines/whitespace from task description before using as PR title - Remove unused useWorktreeWasSetByTaskType variable
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Normalize openPR→useWorktree invariant bidirectionally (openPR:true forces useWorktree:true instead of being silently cleared) - Fix toggleAppMetadataOverride rule ordering to respect user's toggle intent - Propagate openPR flag in direct-spawn completion path - Use isTruthyMeta for simplify/reviewLoop prompt checks
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…antics - addTask persists both true and false for useWorktree/openPR/simplify/reviewLoop so users can explicitly override app defaults (isTruthyMeta/isFalsyMeta handle string round-trip from TASKS.md) - ScheduleTab toggleMetadataField keeps only true entries (false = inherit default)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The task parser lowercased all metadata keys and the serializer capitalized the first character, so openPR→OpenPR→openpr on round-trip. This broke camelCase keys like openPR, useWorktree, reviewLoop after reload. Now both serializer and parser preserve the original key casing.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lowercase first character of parsed keys to handle legacy Title-Case keys (Context→context, App→app) while preserving camelCase keys (openPR, useWorktree). Adds test for camelCase key preservation.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- PR targets baseBranch (not devBranch) since worktrees are created from it - Prompt says "source branch" instead of "default branch" for auto-merge - Updated taskParser docstring for metadata key format
Summary
useWorktreeandopenPRdefaultOpenPRapp-level setting alongside existingdefaultUseWorktreeopenPRis enabled, the system automatically pushes the branch and creates a PR on task completionuseWorktreeis enabled (no PR), the worktree branch auto-merges to default branch on completionBehavior Matrix
Changes
openPRto allowed task metadata keys,defaultOpenPRto app schemaapplyAppWorktreeDefaulthandles both flags with proper cascadingopenPR: trueopenPRthrough task creation routeTest plan