From ab001867a831afa556349d590226a4fbe43336db Mon Sep 17 00:00:00 2001 From: aaight Date: Mon, 11 May 2026 18:15:00 +0200 Subject: [PATCH 1/2] fix(pm): prevent Linear backlog selection from team listings (#1347) Co-authored-by: Cascade Bot --- docs/architecture/04-agent-system.md | 12 ++++ docs/architecture/07-gadgets.md | 8 +++ docs/architecture/08-config-credentials.md | 10 ++++ src/agents/prompts/index.ts | 32 ++++++++-- src/agents/prompts/templates/alerting.eta | 4 +- .../prompts/templates/backlog-manager.eta | 14 ++--- src/agents/prompts/templates/splitting.eta | 13 +++-- src/agents/shared/promptContext.ts | 32 +++++----- src/backends/secretOrchestrator.ts | 2 +- src/cli/pm/list-work-items.ts | 5 +- src/gadgets/pm/ListWorkItems.ts | 5 +- src/gadgets/pm/core/listWorkItems.ts | 29 +++++++++- src/gadgets/pm/core/moveWorkItem.ts | 58 ++++++++++++++----- src/gadgets/pm/definitions.ts | 16 ++++- src/pm/linear/adapter.ts | 36 ++++++++++-- src/pm/types.ts | 6 +- src/sentry/integration.ts | 2 +- tests/unit/agents/prompts.test.ts | 33 ++++++++--- .../unit/agents/shared/promptContext.test.ts | 21 ++++++- tests/unit/cli/pm/pm-commands.test.ts | 9 ++- .../gadgets/pm/core/listWorkItems.test.ts | 45 +++++++++++++- .../unit/gadgets/pm/core/moveWorkItem.test.ts | 42 +++++++++++++- tests/unit/gadgets/pm/definitions.test.ts | 6 +- tests/unit/pm/linear-adapter.test.ts | 8 +++ tests/unit/pm/linear/adapter.test.ts | 33 +++++++++++ .../projects/integration-alerting-tab.tsx | 6 +- 26 files changed, 403 insertions(+), 84 deletions(-) diff --git a/docs/architecture/04-agent-system.md b/docs/architecture/04-agent-system.md index 32ef886fb..8cbb6144a 100644 --- a/docs/architecture/04-agent-system.md +++ b/docs/architecture/04-agent-system.md @@ -200,6 +200,18 @@ Managed via: - Dashboard: Settings > Prompts - CLI: `cascade prompts set-partial`, `cascade prompts reset-partial` +### PM prompt context + +Pipeline prompts receive separate PM identifiers for selection and creation: + +| Variable | Purpose | +|----------|---------| +| `backlogStatusId` | Provider-native BACKLOG workflow state/list used for backlog selection and `MoveWorkItem.expectedSourceState` | +| `workItemCreateContainerId` | Provider-native container used for `CreateWorkItem` | +| `backlogListId` | Deprecated compatibility alias for older custom prompts | + +For Trello, BACKLOG is a list, so `backlogStatusId` and `workItemCreateContainerId` are both the backlog list ID. For JIRA, `backlogStatusId` is `jira.statuses.backlog` and creation uses `jira.projectKey`. For Linear, `backlogStatusId` is `linear.statuses.backlog` and creation uses `linear.teamId`; backlog-manager must not use the Linear team ID to discover candidate backlog issues. + ## Hooks ### Trailing hooks diff --git a/docs/architecture/07-gadgets.md b/docs/architecture/07-gadgets.md index 9875ff4bd..f9167397f 100644 --- a/docs/architecture/07-gadgets.md +++ b/docs/architecture/07-gadgets.md @@ -71,6 +71,14 @@ Todos are stored in `.claude/todos.json` within the repo working directory. PM gadgets use the active `PMProvider` from `AsyncLocalStorage` context, making them provider-agnostic. +`ListWorkItems` accepts either a provider-native `containerId` or a CASCADE status filter. Prefer status filtering for pipeline stages: + +```bash +cascade-tools pm list-work-items --status backlog +``` + +Status filtering calls `provider.listWorkItems(undefined, { status: "backlog" })`, so each provider resolves the configured native workflow state/list. This matters for Linear: `teamId` is a creation/search container and can include unmapped states, while `linear.statuses.backlog` is the only valid backlog selection state. Backlog-manager rejects unfiltered container-only listing to avoid selecting issues from unmapped Linear states such as Ideas. + `ReportFriction` is intentionally narrower than general PM write access. It lets agents file incidental papercuts in tooling, environment, permissions, dependencies, tests, PM data, or SCM data without exposing `CreateWorkItem` / `MoveWorkItem` directly. The CLI form is: ```bash diff --git a/docs/architecture/08-config-credentials.md b/docs/architecture/08-config-credentials.md index e3a7923be..3812c2f9c 100644 --- a/docs/architecture/08-config-credentials.md +++ b/docs/architecture/08-config-credentials.md @@ -67,6 +67,16 @@ PM provider config maps CASCADE lifecycle concepts onto provider-native lists or If the slot is not configured, `ReportFriction` records the report in the sidecar and returns a non-fatal `queued_slot_missing` result with operator guidance. No run should fail solely because the friction slot is missing. +Backlog selection and work-item creation use different native concepts: + +| Provider | Backlog selection | Work-item creation | +|----------|-------------------|--------------------| +| Trello | `lists.backlog` list ID | same backlog list ID | +| JIRA | `statuses.backlog` status name/ID | `projectKey` | +| Linear | `statuses.backlog` workflow state UUID | `teamId` | + +For Linear, `teamId` is not a backlog state. It is the container used when creating or searching issues, and unfiltered team listing may include unmapped workflow states such as Ideas. Pipeline snapshot and backlog-manager paths use status-aware listing (`ListWorkItems --status backlog`) so only the configured `linear.statuses.backlog` state is eligible for selection. + `maxInFlightItems` is enforced at two points: (a) the `backlog-manager` chain gates (won't auto-pull from BACKLOG when at capacity) and (b) the PM `status-changed` triggers (won't fire `implementation` when a card is moved diff --git a/src/agents/prompts/index.ts b/src/agents/prompts/index.ts index 7617c6ba1..5cfeb953b 100644 --- a/src/agents/prompts/index.ts +++ b/src/agents/prompts/index.ts @@ -41,14 +41,19 @@ export interface PromptContext { workItemNounPluralCap?: string; // "Cards" or "Issues" pmName?: string; // "Trello" or "JIRA" - // PM list/column IDs + // PM list/status/container IDs + /** @deprecated Use backlogStatusId for backlog selection and workItemCreateContainerId for creation. */ backlogListId?: string; + /** Provider-native workflow state/list ID for BACKLOG selection. */ + backlogStatusId?: string; + /** Provider-native container ID for creating new work items. */ + workItemCreateContainerId?: string; /** * Value to pass as `expectedSourceState` to the MoveWorkItem gadget * when moving an item out of BACKLOG. Per-provider: * - Trello: backlog list ID (matches `WorkItem.status: card.idList`) * - JIRA: "Backlog" status name - * - Linear: "Backlog" workflow-state name + * - Linear: backlog workflow-state ID */ backlogSourceLabel?: string; todoListId?: string; @@ -314,18 +319,33 @@ export function getTemplateVariables(): Array<{ { name: 'workItemId', group: 'Common', description: 'Work item ID' }, { name: 'workItemUrl', group: 'Common', description: 'Work item URL' }, { name: 'projectId', group: 'Common', description: 'Project identifier' }, - { name: 'pmType', group: 'PM', description: 'PM type: trello or jira' }, + { name: 'pmType', group: 'PM', description: 'PM type: trello, jira, or linear' }, { name: 'workItemNoun', group: 'PM', description: 'card or issue' }, { name: 'workItemNounPlural', group: 'PM', description: 'cards or issues' }, { name: 'workItemNounCap', group: 'PM', description: 'Card or Issue' }, { name: 'workItemNounPluralCap', group: 'PM', description: 'Cards or Issues' }, - { name: 'pmName', group: 'PM', description: 'Trello or JIRA' }, - { name: 'backlogListId', group: 'PM Lists', description: 'Backlog list/column ID' }, + { name: 'pmName', group: 'PM', description: 'Trello, JIRA, or Linear' }, + { + name: 'backlogListId', + group: 'PM Lists', + description: + 'Deprecated backlog selector alias; use backlogStatusId or workItemCreateContainerId', + }, + { + name: 'backlogStatusId', + group: 'PM Lists', + description: 'Provider-native BACKLOG workflow state/list ID for selection and listing', + }, + { + name: 'workItemCreateContainerId', + group: 'PM Lists', + description: 'Provider-native container ID for CreateWorkItem', + }, { name: 'backlogSourceLabel', group: 'PM Lists', description: - 'Value to pass as MoveWorkItem `expectedSourceState` for items in BACKLOG (provider-correct: Trello list ID, JIRA/Linear status name).', + 'Value to pass as MoveWorkItem `expectedSourceState` for items in BACKLOG (provider-correct: Trello list ID, JIRA status name, Linear status ID).', }, { name: 'todoListId', group: 'PM Lists', description: 'TODO list/column ID' }, { name: 'inProgressListId', group: 'PM Lists', description: 'In Progress list/column ID' }, diff --git a/src/agents/prompts/templates/alerting.eta b/src/agents/prompts/templates/alerting.eta index d29a7f20e..876ce11d1 100644 --- a/src/agents/prompts/templates/alerting.eta +++ b/src/agents/prompts/templates/alerting.eta @@ -48,7 +48,7 @@ If you can't confirm a root cause after a reasonable depth (typically 5-10 file Decide where to report based on the trigger context: - If the trigger context provided an **existing work item** (`workItemId` is set, e.g. `<%= it.workItemId || 'NOT_SET' %>`), comment on that work item rather than creating a new one. Reason: the existing work item is presumably the bug's tracking issue, and a comment threads naturally with prior investigation. Use `PostComment` with the structure below. -- Otherwise, if a **backlog list ID** is configured (`backlogListId` is set, e.g. `<%= it.backlogListId || 'NOT_SET' %>`), create a new bug investigation work item via `CreateWorkItem` in that backlog. +- Otherwise, if a **work-item creation container** is configured (`workItemCreateContainerId` is set, e.g. `<%= it.workItemCreateContainerId || it.backlogListId || 'NOT_SET' %>`), create a new bug investigation work item via `CreateWorkItem` in that container. - If neither is configured, log the investigation summary in your final response and exit. The operator will see it in the run record. Then call `Finish` to terminate the run. @@ -93,7 +93,7 @@ You are done when one of these is true: 1. You have created a bug investigation work item OR commented on an existing work item with a root-cause summary, AND the response contains the `Sentry issue:` link. 2. You have honestly determined the investigation cannot proceed (insufficient data, third-party-only stacktrace) and you have recorded that finding via the PM tool with a clear "needs further investigation" framing. -Do **not** terminate without filing or commenting unless neither a work item id nor a backlog list id is available — in which case your final response IS the report. +Do **not** terminate without filing or commenting unless neither a work item id nor a creation container is available — in which case your final response IS the report. After filing, call `Finish`. diff --git a/src/agents/prompts/templates/backlog-manager.eta b/src/agents/prompts/templates/backlog-manager.eta index 63c90f60c..eb8e4303d 100644 --- a/src/agents/prompts/templates/backlog-manager.eta +++ b/src/agents/prompts/templates/backlog-manager.eta @@ -1,9 +1,9 @@ You are a pipeline flow manager responsible for keeping work moving through the development process. -## <%= it.pmName || 'PM' %> List IDs +## <%= it.pmName || 'PM' %> Pipeline IDs -Use these EXACT IDs when calling `ListWorkItems` and `MoveWorkItem`: -- BACKLOG: `<%= it.backlogListId || 'NOT_CONFIGURED' %>` +Use these EXACT IDs when calling `MoveWorkItem`. BACKLOG is a workflow state/list, not necessarily a creation/search container: +- BACKLOG_STATUS_ID: `<%= it.backlogStatusId || it.backlogListId || 'NOT_CONFIGURED' %>` - TODO: `<%= it.todoListId || 'NOT_CONFIGURED' %>` - IN_PROGRESS: `<%= it.inProgressListId || 'NOT_CONFIGURED' %>` - IN_REVIEW: `<%= it.inReviewListId || 'NOT_CONFIGURED' %>` @@ -42,7 +42,7 @@ Note: DONE and MERGED <%= it.workItemNounPlural || 'cards' %> are completed work When the active pipeline has capacity: -1. **Use pre-loaded BACKLOG data** from the Pipeline Snapshot — full details (title, description, checklists, comments) are already available. No need to call `ListWorkItems` or `ReadWorkItem` for BACKLOG <%= it.workItemNounPlural || 'cards' %>. +1. **Use pre-loaded BACKLOG data** from the Pipeline Snapshot — full details (title, description, checklists, comments) are already available. No need to call `ListWorkItems` or `ReadWorkItem` for BACKLOG <%= it.workItemNounPlural || 'cards' %>. If you must verify the backlog list, call `ListWorkItems` with `status: "backlog"`, never with a provider container ID. 2. **Review each <%= it.workItemNoun || 'card' %> from the snapshot** to understand: - Title and description - Checklists and acceptance criteria @@ -59,7 +59,7 @@ When the active pipeline has capacity: - <%= it.workItemNounPluralCap || 'Cards' %> that don't reference incomplete work <% if ((it.maxInFlightItems ?? 1) > 1) { %> - **Conflict Awareness**: When selecting multiple <%= it.workItemNounPlural || 'cards' %>, review in-flight work descriptions to minimize file-level conflicts between simultaneously active <%= it.workItemNounPlural || 'cards' %>. Prefer <%= it.workItemNounPlural || 'cards' %> that touch different areas of the codebase. <% } %>5. **Post a comment** on each selected <%= it.workItemNoun || 'card' %> explaining the selection -6. **Move the selected <%= it.workItemNoun || 'card' %>(s)** using `MoveWorkItem` with the TODO list ID as destination AND `expectedSourceState: <%= it.backlogSourceLabel || 'Backlog' %>`. The `expectedSourceState` guard is **mandatory** — it aborts the move if a parallel run already moved the <%= it.workItemNoun || 'card' %> out of BACKLOG, preventing duplicate downstream implementation runs. +6. **Move the selected <%= it.workItemNoun || 'card' %>(s)** using `MoveWorkItem` with the TODO list ID as destination AND `expectedSourceState: <%= it.backlogSourceLabel || it.backlogStatusId || 'Backlog' %>`. The `expectedSourceState` guard is **mandatory** — it aborts the move if a parallel run already moved the <%= it.workItemNoun || 'card' %> out of BACKLOG, preventing duplicate downstream implementation runs. ## Comment Format @@ -88,7 +88,7 @@ Manual intervention may be needed to unblock the backlog. ## Gadgets Available **<%= it.pmName || 'PM System' %> Operations:** -- `ListWorkItems` - List all <%= it.workItemNounPlural || 'cards' %> in a specific list/column +- `ListWorkItems` - List <%= it.workItemNounPlural || 'cards' %> by configured status (use `status: "backlog"` for BACKLOG verification) - `ReadWorkItem` - Read <%= it.workItemNoun || 'card' %> details (title, description, comments, checklists) - `UpdateWorkItem` - Update <%= it.workItemNoun || 'card' %> title, description, or labels (NOT for moving) - `MoveWorkItem` - Move <%= it.workItemNoun || 'card' %> to a different list/status (use to move to TODO) @@ -96,7 +96,7 @@ Manual intervention may be needed to unblock the backlog. ## Rules -- **HARD CONSTRAINT — NEVER MOVE A <%= it.workItemNounCap || 'CARD' %> NOT IN BACKLOG.** The only valid source list is BACKLOG, and the only valid destination is TODO. Never move <%= it.workItemNounPlural || 'cards' %> from SPLITTING, PLANNING, TODO, IN_PROGRESS, IN_REVIEW, DONE, or MERGED — those <%= it.workItemNounPlural || 'cards' %> are already mid-pipeline and another agent or human owns their state. If you don't see a <%= it.workItemNoun || 'card' %> in the BACKLOG section of the Pipeline Snapshot, you CANNOT select it. NEVER call `ListWorkItems` against non-BACKLOG containers to discover candidates — the snapshot's BACKLOG section is the only valid source. If the snapshot is missing or its BACKLOG section is empty, ABORT — do not improvise by listing other lists. +- **HARD CONSTRAINT — NEVER MOVE A <%= it.workItemNounCap || 'CARD' %> NOT IN BACKLOG.** The only valid source state/list is BACKLOG, and the only valid destination is TODO. Never move <%= it.workItemNounPlural || 'cards' %> from SPLITTING, PLANNING, TODO, IN_PROGRESS, IN_REVIEW, DONE, or MERGED — those <%= it.workItemNounPlural || 'cards' %> are already mid-pipeline and another agent or human owns their state. If you don't see a <%= it.workItemNoun || 'card' %> in the BACKLOG section of the Pipeline Snapshot, you CANNOT select it. NEVER call `ListWorkItems` against provider containers to discover candidates — the snapshot's BACKLOG section is the only valid source. If the snapshot is missing and you must verify, use `ListWorkItems` with `status: "backlog"` only. If the BACKLOG section is empty, ABORT — do not improvise by listing other lists. - ALWAYS check pipeline status FIRST before scanning the backlog - NEVER move <%= it.workItemNounPlural || 'cards' %> if the active pipeline is at capacity (<%= it.maxInFlightItems ?? 1 %> item(s)) - EXIT SILENTLY if pipeline is at capacity - do not post comments diff --git a/src/agents/prompts/templates/splitting.eta b/src/agents/prompts/templates/splitting.eta index 3614d0a62..1058adaec 100644 --- a/src/agents/prompts/templates/splitting.eta +++ b/src/agents/prompts/templates/splitting.eta @@ -2,7 +2,7 @@ You are a very experienced senior technical product manager breaking down work i CRITICAL: 1. DO NOT IMPLEMENT - Focus on breaking down the <%= it.workItemNoun || 'card' %> into user stories. -2. CREATE NEW <%= (it.workItemNounPluralCap || 'Cards').toUpperCase() %> - Use CreateWorkItem to create story <%= it.workItemNounPlural || 'cards' %> in the BACKLOG list. +2. CREATE NEW <%= (it.workItemNounPluralCap || 'Cards').toUpperCase() %> - Use CreateWorkItem to create story <%= it.workItemNounPlural || 'cards' %> in the configured creation container. 3. DO NOT UPDATE the original <%= it.workItemNoun || 'card' %> description. 4. ONLY ASK QUESTIONS if there's genuine ambiguity that blocks progress. 5. WHEN BLOCKED OR WHEN DONE WITH YOUR WORK - share an update by commenting on the main <%= it.workItemNoun || 'card' %> with info what you've done. @@ -82,7 +82,8 @@ You are running in a cloned copy of the project repository. Before creating stor ## Context Variables -- BACKLOG_LIST_ID: <%= it.backlogListId || 'NOT_CONFIGURED' %> +- WORK_ITEM_CREATE_CONTAINER_ID: <%= it.workItemCreateContainerId || it.backlogListId || 'NOT_CONFIGURED' %> +- BACKLOG_STATUS_ID: <%= it.backlogStatusId || it.backlogListId || 'NOT_CONFIGURED' %> - PROCESSED_LABEL_ID: <%= it.processedLabelId || 'NOT_CONFIGURED' %> ## Your Task @@ -97,7 +98,7 @@ You are running in a cloned copy of the project repository. Before creating stor - Each story should be independently valuable - Order stories by dependency (foundational first) 4. **Create story <%= it.workItemNounPlural || 'cards' %>** using `CreateWorkItem`: - - Use the BACKLOG list ID provided in context + - Use WORK_ITEM_CREATE_CONTAINER_ID as the `containerId` - Write clear user story titles - Include TLDR, acceptance criteria, and technical notes in description (use emoji formatting) - **IMPORTANT:** Save the returned URL for each <%= it.workItemNoun || 'card' %> (e.g., `<%= it.pmType === 'jira' ? 'https://your-instance.atlassian.net/browse/PROJ-123' : 'https://trello.com/c/abc123' %>`) @@ -197,9 +198,9 @@ Each story is independent, valuable, and testable. ## Gadgets Available - `ReadWorkItem` - Read <%= it.workItemNoun || 'card' %> details (title, description, comments, labels) -- `CreateWorkItem` - Create new <%= it.workItemNounPlural || 'cards' %> in the BACKLOG list +- `CreateWorkItem` - Create new <%= it.workItemNounPlural || 'cards' %> in the configured creation container - `AddChecklist` - Add an interactive checklist to a <%= it.workItemNoun || 'card' %> (use for acceptance criteria) -- `ListWorkItems` - List all <%= it.workItemNounPlural || 'cards' %> on a list (use to find <%= it.workItemNounPlural || 'cards' %> you created) +- `ListWorkItems` - List <%= it.workItemNounPlural || 'cards' %> by configured status (use `status: "backlog"` to find backlog <%= it.workItemNounPlural || 'cards' %>) - `UpdateWorkItem` - Update <%= it.workItemNoun || 'card' %> title/description, or add labels - `PostComment` - Post a comment on a <%= it.workItemNoun || 'card' %> - `ListDirectory`, `ReadFile`, `RipGrep`, `Tmux` - Explore the codebase @@ -207,7 +208,7 @@ Each story is independent, valuable, and testable. ## Updating Previously Created Stories If the user asks you to update stories you previously created: -1. Use `ListWorkItems` with BACKLOG_LIST_ID to see all <%= it.workItemNounPlural || 'cards' %> in the BACKLOG list +1. Use `ListWorkItems` with `status: "backlog"` to see all <%= it.workItemNounPlural || 'cards' %> in the configured BACKLOG state/list 2. Find the <%= it.workItemNounPlural || 'cards' %> that match what needs to be updated 3. Use `UpdateWorkItem` to modify their title or description 4. Post a comment on the original <%= it.workItemNoun || 'card' %> summarizing what you changed diff --git a/src/agents/shared/promptContext.ts b/src/agents/shared/promptContext.ts index 150f92780..a247e8c40 100644 --- a/src/agents/shared/promptContext.ts +++ b/src/agents/shared/promptContext.ts @@ -7,20 +7,24 @@ function getListIds(project: ProjectConfig) { const trelloConfig = getTrelloConfig(project); const jiraConfig = getJiraConfig(project); const linearConfig = getLinearConfig(project); + const backlogStatusId = + trelloConfig?.lists?.backlog ?? + jiraConfig?.statuses?.backlog ?? + linearConfig?.statuses?.backlog; + const workItemCreateContainerId = + trelloConfig?.lists?.backlog ?? jiraConfig?.projectKey ?? linearConfig?.teamId; return { // Value the agent should pass as `expectedSourceState` when moving an - // item out of BACKLOG. Aligned with what `WorkItem.status` returns - // per provider: - // - Trello: `WorkItem.status` is the list ID → pass list ID. - // - JIRA: `WorkItem.status` is the status name → pass status name. - // - Linear: `WorkItem.status` is the workflow-state name → pass the - // state name. Linear teams default to literal "Backlog"; - // customized teams can still pass it via the prompt path - // since the gadget guard is case-insensitive. - backlogSourceLabel: trelloConfig?.lists?.backlog ?? jiraConfig?.statuses?.backlog ?? 'Backlog', - backlogListId: - trelloConfig?.lists?.backlog ?? jiraConfig?.statuses?.backlog ?? linearConfig?.teamId, + // item out of BACKLOG. It is the provider-native source state/list ID + // where available, and is intentionally not Linear's team container. + backlogSourceLabel: backlogStatusId ?? 'Backlog', + backlogStatusId, + workItemCreateContainerId, + // Deprecated compatibility alias for older custom prompts. Built-in + // prompts use backlogStatusId for listing/move guards and + // workItemCreateContainerId for creation. + backlogListId: backlogStatusId, todoListId: trelloConfig?.lists?.todo ?? jiraConfig?.statuses?.todo ?? linearConfig?.statuses?.todo, inProgressListId: @@ -63,7 +67,7 @@ function getPromptTerminology(pmType: string | undefined) { * building logic including PM-type normalization and work item noun i18n. * * @param alertingResultsContainerId - Optional PM container ID from Sentry integration config. - * Used as a fallback `backlogListId` when no PM backlog is configured on the project. + * Used as a fallback creation container when no PM backlog is configured on the project. * Populated by `secretOrchestrator` for alerting agent runs. */ export function buildPromptContext( @@ -84,8 +88,9 @@ export function buildPromptContext( const listIds = getListIds(project); const terminology = getPromptTerminology(pmProvider?.type); - // Fall back to the Sentry-configured results container when no PM backlog is set. + // Fall back to the Sentry-configured results container when no PM backlog/create container is set. const backlogListId = listIds.backlogListId ?? alertingResultsContainerId; + const workItemCreateContainerId = listIds.workItemCreateContainerId ?? alertingResultsContainerId; return { workItemId, @@ -94,6 +99,7 @@ export function buildPromptContext( baseBranch: project.baseBranch, ...listIds, backlogListId, + workItemCreateContainerId, pmType: pmProvider?.type, ...terminology, maxInFlightItems: project.maxInFlightItems ?? 1, diff --git a/src/backends/secretOrchestrator.ts b/src/backends/secretOrchestrator.ts index 7ac22f7ed..8b97be5d7 100644 --- a/src/backends/secretOrchestrator.ts +++ b/src/backends/secretOrchestrator.ts @@ -67,7 +67,7 @@ export async function buildExecutionPlan( : undefined; // For alerting agents, look up Sentry `resultsContainerId` as a fallback - // backlogListId when no PM backlog status/list is configured on the project. + // creation container when no PM backlog/create target is configured on the project. let alertingResultsContainerId: string | undefined; if (agentType === 'alerting') { try { diff --git a/src/cli/pm/list-work-items.ts b/src/cli/pm/list-work-items.ts index f01767728..02f1c2894 100644 --- a/src/cli/pm/list-work-items.ts +++ b/src/cli/pm/list-work-items.ts @@ -3,5 +3,8 @@ import { listWorkItemsDef } from '../../gadgets/pm/definitions.js'; import { createCLICommand } from '../../gadgets/shared/cliCommandFactory.js'; export default createCLICommand(listWorkItemsDef, async (params) => { - return listWorkItems(params.containerId as string); + return listWorkItems({ + containerId: params.containerId as string | undefined, + status: params.status as string | undefined, + }); }); diff --git a/src/gadgets/pm/ListWorkItems.ts b/src/gadgets/pm/ListWorkItems.ts index af068dfd7..087f62f6b 100644 --- a/src/gadgets/pm/ListWorkItems.ts +++ b/src/gadgets/pm/ListWorkItems.ts @@ -3,5 +3,8 @@ import { listWorkItems } from './core/listWorkItems.js'; import { listWorkItemsDef } from './definitions.js'; export const ListWorkItems = createGadgetClass(listWorkItemsDef, async (params) => { - return listWorkItems(params.containerId as string); + return listWorkItems({ + containerId: params.containerId as string | undefined, + status: params.status as string | undefined, + }); }); diff --git a/src/gadgets/pm/core/listWorkItems.ts b/src/gadgets/pm/core/listWorkItems.ts index 12a60fd87..25da427a1 100644 --- a/src/gadgets/pm/core/listWorkItems.ts +++ b/src/gadgets/pm/core/listWorkItems.ts @@ -1,8 +1,33 @@ import { getPMProvider } from '../../../pm/index.js'; -export async function listWorkItems(containerId: string): Promise { +export interface ListWorkItemsParams { + containerId?: string; + status?: string; +} + +function normalizeListParams(params: string | ListWorkItemsParams): ListWorkItemsParams { + return typeof params === 'string' ? { containerId: params } : params; +} + +function assertListParams(params: ListWorkItemsParams): void { + if (!params.containerId && !params.status) { + throw new Error('Either containerId or status is required.'); + } + if (process.env.CASCADE_AGENT_TYPE === 'backlog-manager' && !params.status) { + throw new Error( + 'Backlog-manager must list work items with a status filter, e.g. status: "backlog".', + ); + } +} + +export async function listWorkItems(params: string | ListWorkItemsParams): Promise { try { - const items = await getPMProvider().listWorkItems(containerId); + const normalized = normalizeListParams(params); + assertListParams(normalized); + + const items = await getPMProvider().listWorkItems(normalized.containerId, { + ...(normalized.status ? { status: normalized.status } : {}), + }); if (items.length === 0) { return 'No work items found.'; diff --git a/src/gadgets/pm/core/moveWorkItem.ts b/src/gadgets/pm/core/moveWorkItem.ts index 8466fae4c..2c3c3be02 100644 --- a/src/gadgets/pm/core/moveWorkItem.ts +++ b/src/gadgets/pm/core/moveWorkItem.ts @@ -1,4 +1,5 @@ import { getPMProvider } from '../../../pm/index.js'; +import type { WorkItem } from '../../../pm/types.js'; export interface MoveWorkItemParams { workItemId: string; @@ -20,25 +21,50 @@ function normalizeStatus(s: string | undefined): string { return (s ?? '').trim().toLowerCase(); } +function matchesStatus(value: string | undefined, expected: string): boolean { + return normalizeStatus(value) === expected; +} + +function isAlreadyInDestination(current: WorkItem, destination: string): boolean { + const currentStatus = normalizeStatus(current.status); + const currentStatusId = normalizeStatus(current.statusId); + return Boolean( + (currentStatus && currentStatus === destination) || + (currentStatusId && currentStatusId === destination), + ); +} + +function matchesExpectedSource(current: WorkItem, expected: string): boolean { + return matchesStatus(current.status, expected) || matchesStatus(current.statusId, expected); +} + +function formatCurrentStatus(current: WorkItem): string { + if (!current.statusId) return current.status ?? 'unknown'; + return `${current.status ?? 'unknown'} (${current.statusId})`; +} + +async function guardedMove(params: MoveWorkItemParams): Promise { + const provider = getPMProvider(); + const current = await provider.getWorkItem(params.workItemId); + const expected = normalizeStatus(params.expectedSourceState); + const destination = normalizeStatus(params.destination); + + if (isAlreadyInDestination(current, destination)) { + return `Work item ${params.workItemId} already in destination state '${current.status ?? current.statusId}' — no-op`; + } + + if (!matchesExpectedSource(current, expected)) { + return `Aborted: work item ${params.workItemId} is in '${formatCurrentStatus(current)}', expected '${params.expectedSourceState}' (likely already moved by a parallel agent — skipping to avoid duplicate downstream work)`; + } + + await provider.moveWorkItem(params.workItemId, params.destination); + return `Work item ${params.workItemId} moved to ${params.destination} successfully`; +} + export async function moveWorkItem(params: MoveWorkItemParams): Promise { try { if (params.expectedSourceState !== undefined) { - const provider = getPMProvider(); - const current = await provider.getWorkItem(params.workItemId); - const currentStatus = normalizeStatus(current.status); - const expected = normalizeStatus(params.expectedSourceState); - const destination = normalizeStatus(params.destination); - - if (currentStatus && currentStatus === destination) { - return `Work item ${params.workItemId} already in destination state '${current.status}' — no-op`; - } - - if (currentStatus !== expected) { - return `Aborted: work item ${params.workItemId} is in '${current.status ?? 'unknown'}', expected '${params.expectedSourceState}' (likely already moved by a parallel agent — skipping to avoid duplicate downstream work)`; - } - - await provider.moveWorkItem(params.workItemId, params.destination); - return `Work item ${params.workItemId} moved to ${params.destination} successfully`; + return await guardedMove(params); } await getPMProvider().moveWorkItem(params.workItemId, params.destination); diff --git a/src/gadgets/pm/definitions.ts b/src/gadgets/pm/definitions.ts index dd1d0e596..782ee25f8 100644 --- a/src/gadgets/pm/definitions.ts +++ b/src/gadgets/pm/definitions.ts @@ -225,13 +225,19 @@ export const reportFrictionDef: ToolDefinition = { export const listWorkItemsDef: ToolDefinition = { name: 'ListWorkItems', description: - 'List all work items in a container (Trello list or JIRA project). Use this to see items you created or to find items to update.', + 'List work items in a container or by CASCADE status. Prefer status filtering for pipeline stages such as backlog.', timeoutMs: 30000, parameters: { containerId: { type: 'string', - describe: 'Container ID — Trello list ID or JIRA project key', - required: true, + describe: 'Container ID — Trello list ID, JIRA project key, or Linear team ID', + optional: true, + }, + status: { + type: 'string', + describe: + 'Optional CASCADE status key to filter by, e.g. backlog, todo, inProgress, inReview, done, merged', + optional: true, }, }, examples: [ @@ -239,6 +245,10 @@ export const listWorkItemsDef: ToolDefinition = { params: { containerId: 'abc123' }, comment: 'List all work items to find ones to update', }, + { + params: { status: 'backlog' }, + comment: 'Safely list configured backlog items across providers', + }, ], }; diff --git a/src/pm/linear/adapter.ts b/src/pm/linear/adapter.ts index e56913cad..af50c0722 100644 --- a/src/pm/linear/adapter.ts +++ b/src/pm/linear/adapter.ts @@ -38,6 +38,32 @@ import type { const DESCRIPTION_VISIBILITY_TIMEOUT_MS = 1_000; const DESCRIPTION_VISIBILITY_POLL_MS = 25; +const CASCADE_STATUS_KEYS = new Set([ + 'backlog', + 'todo', + 'inProgress', + 'inReview', + 'done', + 'merged', + 'cancelled', + 'canceled', + 'splitting', + 'planning', + 'debug', + 'friction', +]); +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + +function resolveLinearStatusFilter( + status: string | undefined, + configStatuses: LinearConfig['statuses'] | undefined, +): string | null | undefined { + if (!status) return undefined; + const mapped = configStatuses?.[status]; + if (mapped) return mapped; + if (CASCADE_STATUS_KEYS.has(status)) return null; + return UUID_RE.test(status) ? status : null; +} export class LinearPMProvider implements PMProvider { readonly type = 'linear' as const; @@ -69,6 +95,7 @@ export class LinearPMProvider implements PMProvider { description: issue.description ?? '', url: issue.url, status: issue.state?.name, + statusId: issue.state?.id, labels: issue.labels.map( (l): WorkItemLabel => ({ id: l.id, @@ -150,14 +177,12 @@ export class LinearPMProvider implements PMProvider { // containerId is the Linear team ID — defaults to config.teamId. const teamId = containerId || this.config.teamId; if (!teamId) return []; + const stateId = resolveLinearStatusFilter(filter?.status, this.config.statuses); + if (stateId === null) return []; const issues = await linearClient.listIssues({ teamId, ...(this.config.projectId ? { projectId: this.config.projectId } : {}), - ...(filter?.status - ? { - stateId: this.config.statuses?.[filter.status] ?? filter.status, - } - : {}), + ...(stateId ? { stateId } : {}), }); return issues.map((issue) => ({ id: issue.identifier || issue.id, @@ -165,6 +190,7 @@ export class LinearPMProvider implements PMProvider { description: issue.description ?? '', url: issue.url, status: issue.state?.name, + statusId: issue.state?.id, labels: issue.labels.map( (l): WorkItemLabel => ({ id: l.id, diff --git a/src/pm/types.ts b/src/pm/types.ts index 7e5ac883b..473779228 100644 --- a/src/pm/types.ts +++ b/src/pm/types.ts @@ -94,7 +94,10 @@ export interface WorkItem { title: string; description: string; url: string; + /** Human-readable provider status/list name when available. */ status?: string; + /** Provider-native workflow state/list ID when available. */ + statusId?: string; labels: WorkItemLabel[]; /** Inline media references parsed from the work item description */ inlineMedia?: MediaReference[]; @@ -157,7 +160,8 @@ export interface ListWorkItemsFilter { * - JIRA: looks up `config.statuses[status]` for the status name in JQL. * - Linear: looks up `config.statuses[status]` for the state UUID. * - * Falls through to literal value when no mapping exists (backwards compat). + * Providers may allow explicit native IDs as an escape hatch, but canonical + * CASCADE keys must resolve through provider config. */ status?: string; } diff --git a/src/sentry/integration.ts b/src/sentry/integration.ts index ddbc14599..712751bb1 100644 --- a/src/sentry/integration.ts +++ b/src/sentry/integration.ts @@ -16,7 +16,7 @@ export interface SentryIntegrationConfig { organizationSlug: string; /** * PM container ID where the alerting agent creates investigation work items. - * Maps to `backlogListId` in the prompt context when no PM backlog is configured. + * Maps to the prompt creation container when no PM backlog is configured. */ resultsContainerId?: string; } diff --git a/tests/unit/agents/prompts.test.ts b/tests/unit/agents/prompts.test.ts index 6a68a161e..8650bec4c 100644 --- a/tests/unit/agents/prompts.test.ts +++ b/tests/unit/agents/prompts.test.ts @@ -64,16 +64,19 @@ describe('getSystemPrompt', () => { it('renders context variables in splitting prompt', () => { const prompt = getSystemPrompt('splitting', { - backlogListId: 'backlog-123', + workItemCreateContainerId: 'team-123', + backlogStatusId: 'state-backlog', processedLabelId: 'label-456', }); - expect(prompt).toContain('BACKLOG_LIST_ID: backlog-123'); + expect(prompt).toContain('WORK_ITEM_CREATE_CONTAINER_ID: team-123'); + expect(prompt).toContain('BACKLOG_STATUS_ID: state-backlog'); expect(prompt).toContain('PROCESSED_LABEL_ID: label-456'); }); it('uses default values when context is not provided', () => { const prompt = getSystemPrompt('splitting'); - expect(prompt).toContain('BACKLOG_LIST_ID: NOT_CONFIGURED'); + expect(prompt).toContain('WORK_ITEM_CREATE_CONTAINER_ID: NOT_CONFIGURED'); + expect(prompt).toContain('BACKLOG_STATUS_ID: NOT_CONFIGURED'); expect(prompt).toContain('PROCESSED_LABEL_ID: NOT_CONFIGURED'); }); @@ -184,6 +187,16 @@ describe('system prompts content', () => { expect(prompt).not.toContain('RipGrep'); }); + it('backlog-manager prompt uses status-aware backlog listing and state guard wording', () => { + const prompt = getSystemPrompt('backlog-manager', { + backlogStatusId: 'state-backlog', + backlogSourceLabel: 'state-backlog', + }); + expect(prompt).toContain('BACKLOG_STATUS_ID: `state-backlog`'); + expect(prompt).toContain('status: "backlog"'); + expect(prompt).not.toContain('Use these EXACT IDs when calling `ListWorkItems`'); + }); + it('backlog-manager prompt uses template variables for PM terminology', () => { const prompt = getSystemPrompt('backlog-manager'); // Default fallback values should be used @@ -282,6 +295,8 @@ describe('system prompts content', () => { const vars = getTemplateVariables(); const names = vars.map((v) => v.name); expect(names).toContain('maxInFlightItems'); + expect(names).toContain('backlogStatusId'); + expect(names).toContain('workItemCreateContainerId'); }); }); @@ -842,8 +857,10 @@ describe('alerting prompt (spec 018)', () => { ).not.toThrow(); }); - it('renders without throwing when only a backlogListId is provided', () => { - expect(() => getSystemPrompt('alerting', { backlogListId: 'list-abc' })).not.toThrow(); + it('renders without throwing when only a creation container is provided', () => { + expect(() => + getSystemPrompt('alerting', { workItemCreateContainerId: 'container-abc' }), + ).not.toThrow(); }); it('contains all three phase markers in order', () => { @@ -881,10 +898,10 @@ describe('alerting prompt (spec 018)', () => { expect(prompt).toContain('WI-1234'); }); - it('directs creating a backlog work item when only backlogListId is provided', () => { - const prompt = getSystemPrompt('alerting', { backlogListId: 'list-abc' }); + it('directs creating a backlog work item when only creation container is provided', () => { + const prompt = getSystemPrompt('alerting', { workItemCreateContainerId: 'container-abc' }); expect(prompt).toMatch(/create/i); - expect(prompt).toContain('list-abc'); + expect(prompt).toContain('container-abc'); }); it('does not contain engine-specific tool-call syntax', () => { diff --git a/tests/unit/agents/shared/promptContext.test.ts b/tests/unit/agents/shared/promptContext.test.ts index 5016b4fcf..fa33b8c51 100644 --- a/tests/unit/agents/shared/promptContext.test.ts +++ b/tests/unit/agents/shared/promptContext.test.ts @@ -175,6 +175,8 @@ describe('buildPromptContext', () => { }); const ctx = buildPromptContext('PROJ-1', jiraProject as never); expect(ctx.backlogListId).toBe('Backlog'); + expect(ctx.backlogStatusId).toBe('Backlog'); + expect(ctx.workItemCreateContainerId).toBe('PROJ'); expect(ctx.todoListId).toBe('To Do'); expect(ctx.inProgressListId).toBe('In Progress'); expect(ctx.inReviewListId).toBe('In Review'); @@ -193,6 +195,8 @@ describe('buildPromptContext', () => { }); const ctx = buildPromptContext('PROJ-1', jiraProject as never); expect(ctx.backlogListId).toBeUndefined(); + expect(ctx.backlogStatusId).toBeUndefined(); + expect(ctx.workItemCreateContainerId).toBe('PROJ'); expect(ctx.todoListId).toBeUndefined(); expect(ctx.inProgressListId).toBeUndefined(); expect(ctx.inReviewListId).toBeUndefined(); @@ -242,7 +246,7 @@ describe('buildPromptContext', () => { expect(ctx.workItemUrl).toBe('https://linear.app/myorg/issue/TEAM-123'); }); - it('sets backlogListId to teamId (containerId for CreateWorkItem)', () => { + it('separates Linear backlog status ID from creation team container', () => { const linearProject = makeProject({ trello: undefined, pm: { type: 'linear' }, @@ -252,7 +256,10 @@ describe('buildPromptContext', () => { }, }); const ctx = buildPromptContext('TEAM-1', linearProject as never); - expect(ctx.backlogListId).toBe('team-abc'); + expect(ctx.backlogListId).toBe('state-bl-uuid'); + expect(ctx.backlogStatusId).toBe('state-bl-uuid'); + expect(ctx.backlogSourceLabel).toBe('state-bl-uuid'); + expect(ctx.workItemCreateContainerId).toBe('team-abc'); }); it('sets other pipeline list IDs from Linear statuses', () => { @@ -272,6 +279,8 @@ describe('buildPromptContext', () => { }, }); const ctx = buildPromptContext('TEAM-1', linearProject as never); + expect(ctx.backlogStatusId).toBe('Backlog'); + expect(ctx.workItemCreateContainerId).toBe('team-abc'); expect(ctx.todoListId).toBe('Todo'); expect(ctx.inProgressListId).toBe('In Progress'); expect(ctx.inReviewListId).toBe('In Review'); @@ -288,7 +297,9 @@ describe('buildPromptContext', () => { }, }); const ctx = buildPromptContext('TEAM-1', linearProject as never); - expect(ctx.backlogListId).toBe('team-abc'); + expect(ctx.backlogListId).toBeUndefined(); + expect(ctx.backlogStatusId).toBeUndefined(); + expect(ctx.workItemCreateContainerId).toBe('team-abc'); expect(ctx.todoListId).toBeUndefined(); expect(ctx.inProgressListId).toBeUndefined(); expect(ctx.inReviewListId).toBeUndefined(); @@ -500,6 +511,7 @@ describe('buildPromptContext', () => { 'sentry-container-id', ); expect(ctx.backlogListId).toBe('sentry-container-id'); + expect(ctx.workItemCreateContainerId).toBe('sentry-container-id'); }); it('PM backlogListId takes precedence over alertingResultsContainerId', () => { @@ -513,6 +525,7 @@ describe('buildPromptContext', () => { ); // makeProject has trello.lists.backlog = 'list-backlog' expect(ctx.backlogListId).toBe('list-backlog'); + expect(ctx.workItemCreateContainerId).toBe('list-backlog'); }); it('alertingResultsContainerId is ignored when it is undefined', () => { @@ -526,6 +539,7 @@ describe('buildPromptContext', () => { ); // still gets backlog from PM config expect(ctx.backlogListId).toBe('list-backlog'); + expect(ctx.workItemCreateContainerId).toBe('list-backlog'); }); it('backlogListId remains undefined when neither PM backlog nor alertingResultsContainerId is set', () => { @@ -545,6 +559,7 @@ describe('buildPromptContext', () => { }); const ctx = buildPromptContext('card123', projectWithoutBacklog as never); expect(ctx.backlogListId).toBeUndefined(); + expect(ctx.workItemCreateContainerId).toBeUndefined(); }); }); diff --git a/tests/unit/cli/pm/pm-commands.test.ts b/tests/unit/cli/pm/pm-commands.test.ts index d9af24b00..2bb76ddcc 100644 --- a/tests/unit/cli/pm/pm-commands.test.ts +++ b/tests/unit/cli/pm/pm-commands.test.ts @@ -188,7 +188,14 @@ describe('ListWorkItems command', () => { const cmd = new ListWorkItems(['--containerId', 'list-456'], makeMockConfig() as never); await cmd.run(); - expect(listWorkItems).toHaveBeenCalledWith('list-456'); + expect(listWorkItems).toHaveBeenCalledWith({ containerId: 'list-456', status: undefined }); + }); + + it('passes status to listWorkItems', async () => { + const cmd = new ListWorkItems(['--status', 'backlog'], makeMockConfig() as never); + await cmd.run(); + + expect(listWorkItems).toHaveBeenCalledWith({ containerId: undefined, status: 'backlog' }); }); it('outputs JSON success result', async () => { diff --git a/tests/unit/gadgets/pm/core/listWorkItems.test.ts b/tests/unit/gadgets/pm/core/listWorkItems.test.ts index b11f77f65..ba892e948 100644 --- a/tests/unit/gadgets/pm/core/listWorkItems.test.ts +++ b/tests/unit/gadgets/pm/core/listWorkItems.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createMockPMProvider } from '../../../../helpers/mockPMProvider.js'; @@ -11,14 +11,57 @@ vi.mock('../../../../../src/pm/index.js', () => ({ import { listWorkItems } from '../../../../../src/gadgets/pm/core/listWorkItems.js'; describe('listWorkItems', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + it('returns "No work items found." when list is empty', async () => { mockProvider.listWorkItems.mockResolvedValue([]); const result = await listWorkItems('list1'); + expect(mockProvider.listWorkItems).toHaveBeenCalledWith('list1', {}); expect(result).toBe('No work items found.'); }); + it('passes status filter without requiring a container ID', async () => { + mockProvider.listWorkItems.mockResolvedValue([]); + + await listWorkItems({ status: 'backlog' }); + + expect(mockProvider.listWorkItems).toHaveBeenCalledWith(undefined, { status: 'backlog' }); + }); + + it('passes both containerId and status when supplied', async () => { + mockProvider.listWorkItems.mockResolvedValue([]); + + await listWorkItems({ containerId: 'team-1', status: 'todo' }); + + expect(mockProvider.listWorkItems).toHaveBeenCalledWith('team-1', { status: 'todo' }); + }); + + it('rejects missing containerId and status', async () => { + await expect(listWorkItems({})).rejects.toThrow( + 'Error listing work items: Either containerId or status is required.', + ); + }); + + it('rejects unfiltered backlog-manager listing', async () => { + const originalAgentType = process.env.CASCADE_AGENT_TYPE; + process.env.CASCADE_AGENT_TYPE = 'backlog-manager'; + try { + await expect(listWorkItems({ containerId: 'team-1' })).rejects.toThrow( + 'Backlog-manager must list work items with a status filter', + ); + } finally { + if (originalAgentType === undefined) { + delete process.env.CASCADE_AGENT_TYPE; + } else { + process.env.CASCADE_AGENT_TYPE = originalAgentType; + } + } + }); + it('formats work items with title, id, url', async () => { mockProvider.listWorkItems.mockResolvedValue([ { diff --git a/tests/unit/gadgets/pm/core/moveWorkItem.test.ts b/tests/unit/gadgets/pm/core/moveWorkItem.test.ts index 480c95b01..a9a54f9e8 100644 --- a/tests/unit/gadgets/pm/core/moveWorkItem.test.ts +++ b/tests/unit/gadgets/pm/core/moveWorkItem.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createMockPMProvider } from '../../../../helpers/mockPMProvider.js'; @@ -11,6 +11,10 @@ vi.mock('../../../../../src/pm/index.js', () => ({ import { moveWorkItem } from '../../../../../src/gadgets/pm/core/moveWorkItem.js'; describe('moveWorkItem', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + it('calls provider.moveWorkItem with correct args and returns success message', async () => { mockProvider.moveWorkItem.mockResolvedValue(undefined); @@ -78,6 +82,24 @@ describe('moveWorkItem', () => { expect(result).toContain('moved'); }); + it('proceeds with move when current statusId matches expectedSourceState', async () => { + mockProvider.getWorkItem.mockResolvedValue({ + ...baseItem, + status: 'Ready', + statusId: 'state-backlog', + }); + mockProvider.moveWorkItem.mockResolvedValue(undefined); + + const result = await moveWorkItem({ + workItemId: 'MNG-538', + destination: 'state-todo', + expectedSourceState: 'state-backlog', + }); + + expect(mockProvider.moveWorkItem).toHaveBeenCalledWith('MNG-538', 'state-todo'); + expect(result).toContain('moved'); + }); + it('aborts move when current status differs from expectedSourceState', async () => { mockProvider.getWorkItem.mockResolvedValue({ ...baseItem, @@ -96,6 +118,24 @@ describe('moveWorkItem', () => { expect(result).toContain('Backlog'); }); + it('aborts move when Linear issue is in an unmapped Ideas statusId', async () => { + mockProvider.getWorkItem.mockResolvedValue({ + ...baseItem, + status: 'Ideas', + statusId: 'state-ideas', + }); + + const result = await moveWorkItem({ + workItemId: 'MNG-700', + destination: 'state-todo', + expectedSourceState: 'state-backlog', + }); + + expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); + expect(result).toContain('Ideas (state-ideas)'); + expect(result).toContain('state-backlog'); + }); + it('matches expectedSourceState case-insensitively (Linear vs Trello casing drift)', async () => { mockProvider.getWorkItem.mockResolvedValue({ ...baseItem, diff --git a/tests/unit/gadgets/pm/definitions.test.ts b/tests/unit/gadgets/pm/definitions.test.ts index e09a9b441..f71fb5689 100644 --- a/tests/unit/gadgets/pm/definitions.test.ts +++ b/tests/unit/gadgets/pm/definitions.test.ts @@ -248,8 +248,10 @@ describe('PM gadget definitions', () => { // ─── ListWorkItems specific ──────────────────────────────────────────────── describe('listWorkItemsDef', () => { - it('has required containerId parameter', () => { - expect(listWorkItemsDef.parameters.containerId?.required).toBe(true); + it('has optional containerId and status parameters', () => { + expect(listWorkItemsDef.parameters.containerId?.optional).toBe(true); + expect(listWorkItemsDef.parameters.status?.optional).toBe(true); + expect(listWorkItemsDef.parameters.status?.type).toBe('string'); }); }); diff --git a/tests/unit/pm/linear-adapter.test.ts b/tests/unit/pm/linear-adapter.test.ts index 78407a5e8..be83f2468 100644 --- a/tests/unit/pm/linear-adapter.test.ts +++ b/tests/unit/pm/linear-adapter.test.ts @@ -63,6 +63,14 @@ describe('LinearPMProvider.listWorkItems — project scope', () => { expect.objectContaining({ teamId: 'T1', projectId: 'P1', stateId: 'S-BL' }), ); }); + + it('returns [] when a canonical status key is not mapped', async () => { + const spy = vi.spyOn(linearClient, 'listIssues').mockResolvedValue([]); + const provider = new LinearPMProvider(configOf({ projectId: 'P1', statuses: {} })); + const result = await provider.listWorkItems('T1', { status: 'backlog' }); + expect(result).toEqual([]); + expect(spy).not.toHaveBeenCalled(); + }); }); describe('LinearPMProvider.listWorkItems — self-resolution from config', () => { diff --git a/tests/unit/pm/linear/adapter.test.ts b/tests/unit/pm/linear/adapter.test.ts index b2b9f6e16..cc3b7ef95 100644 --- a/tests/unit/pm/linear/adapter.test.ts +++ b/tests/unit/pm/linear/adapter.test.ts @@ -125,6 +125,7 @@ describe('LinearPMProvider', () => { expect(result.description).toBe('A description'); expect(result.url).toBe('https://linear.app/org/issue/TEAM-1'); expect(result.status).toBe('Backlog'); + expect(result.statusId).toBe('state-backlog'); expect(result.labels).toHaveLength(1); expect(result.labels[0]).toEqual({ id: 'label-1', name: 'Bug', color: '#f00' }); }); @@ -377,6 +378,38 @@ describe('LinearPMProvider', () => { expect(mockListIssues).toHaveBeenCalledWith(expect.objectContaining({ teamId: 'team-abc' })); expect(result).toHaveLength(2); + expect(result[0].status).toBe('Backlog'); + expect(result[0].statusId).toBe('state-backlog'); + }); + + it('returns [] for unmapped CASCADE status keys without falling through to Linear stateId', async () => { + const noBacklogProvider = new LinearPMProvider({ + teamId: 'team-abc', + statuses: {}, + }); + + const result = await noBacklogProvider.listWorkItems(undefined, { status: 'backlog' }); + + expect(result).toEqual([]); + expect(mockListIssues).not.toHaveBeenCalled(); + }); + + it('allows UUID-shaped raw state IDs for explicit low-level callers', async () => { + mockListIssues.mockResolvedValue([]); + const rawStateId = '550e8400-e29b-41d4-a716-446655440000'; + + await provider.listWorkItems(undefined, { status: rawStateId }); + + expect(mockListIssues).toHaveBeenCalledWith( + expect.objectContaining({ teamId: 'team-abc', stateId: rawStateId }), + ); + }); + + it('rejects non-UUID unmapped raw status values', async () => { + const result = await provider.listWorkItems(undefined, { status: 'Ideas' }); + + expect(result).toEqual([]); + expect(mockListIssues).not.toHaveBeenCalled(); }); }); diff --git a/web/src/components/projects/integration-alerting-tab.tsx b/web/src/components/projects/integration-alerting-tab.tsx index 713083099..4869ae789 100644 --- a/web/src/components/projects/integration-alerting-tab.tsx +++ b/web/src/components/projects/integration-alerting-tab.tsx @@ -32,7 +32,7 @@ interface ProviderPickerConfig { /** * Which property of the returned discovery item to save as `resultsContainerId`. * - JIRA `states` → `name` (status name — the adapter matches transitions by name). - * - Linear `teams` → `id` (team UUID used directly as `backlogListId`). + * - Linear `teams` → `id` (team UUID used as the alert-created work item container). */ getOptionValue: (item: { id: string; name: string }) => string; } @@ -42,9 +42,9 @@ interface ProviderPickerConfig { * dropdown picker is supported for that provider. * * - JIRA: `states` capability, scoped to the configured project key. - * `backlogListId` for JIRA = a status name (e.g. "Backlog"), so the picker + * Sentry-created issues can be moved to a status by name, so the picker * saves `item.name` not the numeric state ID. - * - Linear: `teams` capability; team ID is correct for `backlogListId`. + * - Linear: `teams` capability; team ID is correct for creating alert work items. * - Trello: no `lists` discovery capability exists in the manifest * (`boards` capability returns boards, not lists within a board). * Trello users must enter the list ID manually. From 79c6de14aa10e281678ddba4a70368cf29169338 Mon Sep 17 00:00:00 2001 From: aaight Date: Mon, 11 May 2026 18:35:13 +0200 Subject: [PATCH 2/2] fix(pm): batch inline checklist creation (#1348) Co-authored-by: Cascade Bot --- src/gadgets/pm/core/addChecklist.ts | 25 ++++++-- src/gadgets/pm/definitions.ts | 6 +- src/integrations/README.md | 6 +- src/pm/_shared/description-mutation-lock.ts | 4 +- src/pm/index.ts | 1 + src/pm/jira/adapter.ts | 27 ++++++++ src/pm/linear/adapter.ts | 27 ++++++++ src/pm/types.ts | 17 +++++ .../unit/gadgets/pm/core/addChecklist.test.ts | 35 ++++++++++- tests/unit/gadgets/pm/definitions.test.ts | 12 ++++ .../_shared/description-mutation-lock.test.ts | 9 ++- tests/unit/pm/jira/adapter.test.ts | 61 ++++++++++++++++++ tests/unit/pm/linear/adapter.test.ts | 62 +++++++++++++++++++ 13 files changed, 278 insertions(+), 14 deletions(-) diff --git a/src/gadgets/pm/core/addChecklist.ts b/src/gadgets/pm/core/addChecklist.ts index 32b13b463..8ed7af098 100644 --- a/src/gadgets/pm/core/addChecklist.ts +++ b/src/gadgets/pm/core/addChecklist.ts @@ -1,4 +1,5 @@ import { getPMProvider } from '../../../pm/index.js'; +import type { ChecklistItemDraft } from '../../../pm/types.js'; export type ChecklistItemInput = string | { name: string; description?: string }; @@ -14,13 +15,27 @@ export async function addChecklist(params: AddChecklistParams): Promise } const provider = getPMProvider(); + const items = params.items.map(normalizeChecklistItem); + + if (typeof provider.createChecklistWithItems === 'function') { + await provider.createChecklistWithItems(params.workItemId, params.checklistName, items); + return successMessage(params.workItemId, params.checklistName, items.length); + } + const checklist = await provider.createChecklist(params.workItemId, params.checklistName); - for (const item of params.items) { - const name = typeof item === 'string' ? item : item.name; - const description = typeof item === 'string' ? undefined : item.description; - await provider.addChecklistItem(checklist.id, name, false, description); + for (const item of items) { + await provider.addChecklistItem(checklist.id, item.name, item.checked, item.description); } - return `Checklist "${params.checklistName}" created with ${params.items.length} items on work item ${params.workItemId}`; + return successMessage(params.workItemId, params.checklistName, items.length); +} + +function normalizeChecklistItem(item: ChecklistItemInput): ChecklistItemDraft { + if (typeof item === 'string') return { name: item, checked: false }; + return { name: item.name, checked: false, description: item.description }; +} + +function successMessage(workItemId: string, checklistName: string, itemCount: number): string { + return `Checklist "${checklistName}" created with ${itemCount} items on work item ${workItemId}`; } diff --git a/src/gadgets/pm/definitions.ts b/src/gadgets/pm/definitions.ts index 782ee25f8..0564a9a57 100644 --- a/src/gadgets/pm/definitions.ts +++ b/src/gadgets/pm/definitions.ts @@ -299,7 +299,7 @@ export const addChecklistDef: ToolDefinition = { name: 'AddChecklist', description: 'Add a checklist with items to a work item. Use this to create interactive checklists for acceptance criteria or implementation steps.', - timeoutMs: 30000, + timeoutMs: 60000, parameters: { workItemId: { type: 'string', @@ -346,7 +346,7 @@ export const pmUpdateChecklistItemDef: ToolDefinition = { name: 'PMUpdateChecklistItem', description: 'Update a checklist item state on a work item. Use this to mark items as complete or incomplete.', - timeoutMs: 15000, + timeoutMs: 60000, parameters: { workItemId: { type: 'string', @@ -381,7 +381,7 @@ export const pmDeleteChecklistItemDef: ToolDefinition = { name: 'PMDeleteChecklistItem', description: 'Delete a checklist item from a work item. For JIRA this deletes the subtask issue. For Trello this removes the checklist item. Use this to remove descoped or invalid plan steps — do NOT mark items as "complete" if they were never done.', - timeoutMs: 15000, + timeoutMs: 60000, parameters: { workItemId: { type: 'string', diff --git a/src/integrations/README.md b/src/integrations/README.md index 228b251bf..4987afd3b 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -285,7 +285,7 @@ If the slot is missing, the materializer returns a skipped result with reason `f ## Checklist implementation by provider -Different PM providers have different native concepts of "checklist". The `PMProvider` interface exposes a uniform API (`getChecklists`, `createChecklist`, `addChecklistItem`, `updateChecklistItem`, `deleteChecklistItem`), but adapters implement them differently: +Different PM providers have different native concepts of "checklist". The `PMProvider` interface exposes a uniform API (`getChecklists`, `createChecklist`, `addChecklistItem`, `updateChecklistItem`, `deleteChecklistItem`) plus an optional bulk creation path (`createChecklistWithItems`), but adapters implement them differently: | Provider | Implementation | Where items live | |---|---|---| @@ -297,7 +297,9 @@ Different PM providers have different native concepts of "checklist". The `PMPro The shared engine that parses, appends, toggles, and removes inline checklist items lives at `src/pm/_shared/inline-checklist.ts` and is consumed by both the Linear and JIRA adapters. -Because Linear and JIRA checklist mutations rewrite the whole description, their adapters serialize the full read/mutate/write operation with `withDescriptionMutationLock(provider, workItemId, fn)` from `src/pm/_shared/description-mutation-lock.ts`. Keep future inline-description mutations inside that guard; otherwise concurrent `cascade-tools pm update-checklist-item` processes can overwrite each other's description snapshots without a provider-side conflict error. Linear mutations must also keep the lock held until `linearClient.getIssue()` observes the markdown just written by `linearClient.updateIssue()`, because Linear can briefly serve stale descriptions after accepting a write. Releasing the lock before read-after-write convergence lets the next queued checklist mutation start from an old snapshot and miss a newly created `###` checklist heading. +Because Linear and JIRA checklist mutations rewrite the whole description, their adapters serialize the full read/mutate/write operation with `withDescriptionMutationLock(provider, workItemId, fn)` from `src/pm/_shared/description-mutation-lock.ts`. Keep future inline-description mutations inside that guard; otherwise concurrent `cascade-tools pm update-checklist-item` processes can overwrite each other's description snapshots without a provider-side conflict error. Initial checklist creation for inline-description providers should implement `createChecklistWithItems` so `AddChecklist` can create the section and all starting rows in one locked description mutation instead of `createChecklist` plus one write per item. Linear mutations must also keep the lock held until `linearClient.getIssue()` observes the markdown just written by `linearClient.updateIssue()`, because Linear can briefly serve stale descriptions after accepting a write. Releasing the lock before read-after-write convergence lets the next queued checklist mutation start from an old snapshot and miss a newly created `###` checklist heading. + +The default description-lock wait budget is intentionally lower than the checklist tool timeout: the lock waits up to 45s, while `AddChecklist`, `PMUpdateChecklistItem`, and `PMDeleteChecklistItem` allow 60s. Keep that relationship when adjusting either value so a queued, legitimate Linear/JIRA checklist mutation is not aborted by the outer tool runner before the lock wait can complete. --- diff --git a/src/pm/_shared/description-mutation-lock.ts b/src/pm/_shared/description-mutation-lock.ts index c6572ae50..ae333ea1c 100644 --- a/src/pm/_shared/description-mutation-lock.ts +++ b/src/pm/_shared/description-mutation-lock.ts @@ -17,7 +17,7 @@ interface LockFileContents { pid: number; } -const DEFAULT_TIMEOUT_MS = 10_000; +export const DEFAULT_DESCRIPTION_MUTATION_LOCK_TIMEOUT_MS = 45_000; const DEFAULT_STALE_MS = 120_000; const DEFAULT_POLL_MS = 25; const LOCK_DIR_ENV = 'CASCADE_DESCRIPTION_MUTATION_LOCK_DIR'; @@ -50,7 +50,7 @@ function resolveOptions( return { lockDir: options.lockDir ?? process.env[LOCK_DIR_ENV] ?? join(tmpdir(), 'cascade-description-locks'), - timeoutMs: options.timeoutMs ?? DEFAULT_TIMEOUT_MS, + timeoutMs: options.timeoutMs ?? DEFAULT_DESCRIPTION_MUTATION_LOCK_TIMEOUT_MS, staleMs: options.staleMs ?? DEFAULT_STALE_MS, pollMs: options.pollMs ?? DEFAULT_POLL_MS, }; diff --git a/src/pm/index.ts b/src/pm/index.ts index e4a531c88..8eb2816f9 100644 --- a/src/pm/index.ts +++ b/src/pm/index.ts @@ -18,6 +18,7 @@ export type { Attachment, Checklist, ChecklistItem, + ChecklistItemDraft, CreateWorkItemConfig, MediaReference, PMProvider, diff --git a/src/pm/jira/adapter.ts b/src/pm/jira/adapter.ts index c31ccd370..137be7aa4 100644 --- a/src/pm/jira/adapter.ts +++ b/src/pm/jira/adapter.ts @@ -12,6 +12,7 @@ import { appendChecklistSection, buildChecklistId, findChecklistNameByHash, + hashChecklistItemId, parseChecklistId, parseInlineChecklists, removeChecklistItem, @@ -23,6 +24,7 @@ import { resolveJiraMediaUrls } from '../media.js'; import type { Attachment, Checklist, + ChecklistItemDraft, CreateWorkItemConfig, ListWorkItemsFilter, PMProvider, @@ -289,6 +291,31 @@ export class JiraPMProvider implements PMProvider { }; } + async createChecklistWithItems( + workItemId: string, + name: string, + items: ChecklistItemDraft[], + ): Promise { + await this.updateDescription(workItemId, (desc) => + appendChecklistSection( + desc, + name, + items.map((item) => ({ name: item.name, checked: item.checked ?? false })), + ), + ); + + return { + id: buildChecklistId(workItemId, name), + name, + workItemId, + items: items.map((item) => ({ + id: hashChecklistItemId(name, item.name), + name: item.name, + complete: item.checked ?? false, + })), + }; + } + async addChecklistItem( checklistId: string, name: string, diff --git a/src/pm/linear/adapter.ts b/src/pm/linear/adapter.ts index af50c0722..df4c791ce 100644 --- a/src/pm/linear/adapter.ts +++ b/src/pm/linear/adapter.ts @@ -17,6 +17,7 @@ import { appendChecklistSection, buildChecklistId, findChecklistNameByHash, + hashChecklistItemId, parseChecklistId, parseInlineChecklists, removeChecklistItem, @@ -28,6 +29,7 @@ import { extractMarkdownImages } from '../media.js'; import type { Attachment, Checklist, + ChecklistItemDraft, CreateWorkItemConfig, ListWorkItemsFilter, PMProvider, @@ -243,6 +245,31 @@ export class LinearPMProvider implements PMProvider { }; } + async createChecklistWithItems( + workItemId: string, + name: string, + items: ChecklistItemDraft[], + ): Promise { + await this.updateDescription(workItemId, (desc) => + appendChecklistSection( + desc, + name, + items.map((item) => ({ name: item.name, checked: item.checked ?? false })), + ), + ); + + return { + id: buildChecklistId(workItemId, name), + name, + workItemId, + items: items.map((item) => ({ + id: hashChecklistItemId(name, item.name), + name: item.name, + complete: item.checked ?? false, + })), + }; + } + async addChecklistItem( checklistId: string, name: string, diff --git a/src/pm/types.ts b/src/pm/types.ts index 473779228..31aa233f1 100644 --- a/src/pm/types.ts +++ b/src/pm/types.ts @@ -135,6 +135,12 @@ export interface ChecklistItem { complete: boolean; } +export interface ChecklistItemDraft { + name: string; + checked?: boolean; + description?: string; +} + export interface Attachment { id: string; name: string; @@ -195,6 +201,17 @@ export interface PMProvider { // Checklists getChecklists(workItemId: string): Promise; createChecklist(workItemId: string, name: string): Promise; + /** + * Optional bulk creation path for providers that rewrite an entire work-item + * description to emulate checklists. Inline-description providers should use + * this to create the checklist section and initial items in one provider-level + * mutation instead of N+1 description rewrites. + */ + createChecklistWithItems?( + workItemId: string, + name: string, + items: ChecklistItemDraft[], + ): Promise; addChecklistItem( checklistId: string, name: string, diff --git a/tests/unit/gadgets/pm/core/addChecklist.test.ts b/tests/unit/gadgets/pm/core/addChecklist.test.ts index f18b29612..8063b528f 100644 --- a/tests/unit/gadgets/pm/core/addChecklist.test.ts +++ b/tests/unit/gadgets/pm/core/addChecklist.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { createMockPMProvider } from '../../../../helpers/mockPMProvider.js'; @@ -10,7 +10,40 @@ vi.mock('../../../../../src/pm/index.js', () => ({ import { addChecklist } from '../../../../../src/gadgets/pm/core/addChecklist.js'; +const providerWithBulk = mockProvider as typeof mockProvider & { + createChecklistWithItems?: ReturnType; +}; + describe('addChecklist', () => { + beforeEach(() => { + vi.clearAllMocks(); + delete providerWithBulk.createChecklistWithItems; + }); + + it('uses provider bulk creation when available', async () => { + providerWithBulk.createChecklistWithItems = vi.fn().mockResolvedValue({ + id: 'cl1', + name: 'My Tasks', + workItemId: 'item1', + items: [], + }); + + const result = await addChecklist({ + workItemId: 'item1', + checklistName: 'My Tasks', + items: ['Task A', { name: 'Task B', description: 'Details' }], + }); + + expect(providerWithBulk.createChecklistWithItems).toHaveBeenCalledTimes(1); + expect(providerWithBulk.createChecklistWithItems).toHaveBeenCalledWith('item1', 'My Tasks', [ + { name: 'Task A', checked: false }, + { name: 'Task B', checked: false, description: 'Details' }, + ]); + expect(mockProvider.createChecklist).not.toHaveBeenCalled(); + expect(mockProvider.addChecklistItem).not.toHaveBeenCalled(); + expect(result).toBe('Checklist "My Tasks" created with 2 items on work item item1'); + }); + it('creates checklist and adds string items', async () => { mockProvider.createChecklist.mockResolvedValue({ id: 'cl1', diff --git a/tests/unit/gadgets/pm/definitions.test.ts b/tests/unit/gadgets/pm/definitions.test.ts index f71fb5689..4971a257c 100644 --- a/tests/unit/gadgets/pm/definitions.test.ts +++ b/tests/unit/gadgets/pm/definitions.test.ts @@ -265,6 +265,10 @@ describe('PM gadget definitions', () => { // ─── AddChecklist specific ───────────────────────────────────────────────── describe('addChecklistDef', () => { + it('keeps timeout above the description mutation lock wait budget', () => { + expect(addChecklistDef.timeoutMs).toBe(60_000); + }); + it('has required workItemId, checklistName, and item parameters', () => { expect(addChecklistDef.parameters.workItemId?.required).toBe(true); expect(addChecklistDef.parameters.checklistName?.required).toBe(true); @@ -278,6 +282,10 @@ describe('PM gadget definitions', () => { // ─── PMUpdateChecklistItem specific ──────────────────────────────────────── describe('pmUpdateChecklistItemDef', () => { + it('keeps timeout above the description mutation lock wait budget', () => { + expect(pmUpdateChecklistItemDef.timeoutMs).toBe(60_000); + }); + it('has required workItemId, checkItemId, and state parameters', () => { expect(pmUpdateChecklistItemDef.parameters.workItemId?.required).toBe(true); expect(pmUpdateChecklistItemDef.parameters.checkItemId?.required).toBe(true); @@ -295,6 +303,10 @@ describe('PM gadget definitions', () => { // ─── PMDeleteChecklistItem specific ──────────────────────────────────────── describe('pmDeleteChecklistItemDef', () => { + it('keeps timeout above the description mutation lock wait budget', () => { + expect(pmDeleteChecklistItemDef.timeoutMs).toBe(60_000); + }); + it('has required workItemId and checkItemId parameters', () => { expect(pmDeleteChecklistItemDef.parameters.workItemId?.required).toBe(true); expect(pmDeleteChecklistItemDef.parameters.checkItemId?.required).toBe(true); diff --git a/tests/unit/pm/_shared/description-mutation-lock.test.ts b/tests/unit/pm/_shared/description-mutation-lock.test.ts index eb22d0830..854bb0193 100644 --- a/tests/unit/pm/_shared/description-mutation-lock.test.ts +++ b/tests/unit/pm/_shared/description-mutation-lock.test.ts @@ -3,7 +3,10 @@ import { readdir, unlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { withDescriptionMutationLock } from '../../../../src/pm/_shared/description-mutation-lock.js'; +import { + DEFAULT_DESCRIPTION_MUTATION_LOCK_TIMEOUT_MS, + withDescriptionMutationLock, +} from '../../../../src/pm/_shared/description-mutation-lock.js'; describe('withDescriptionMutationLock', () => { let lockDir: string; @@ -16,6 +19,10 @@ describe('withDescriptionMutationLock', () => { rmSync(lockDir, { recursive: true, force: true }); }); + it('uses a default wait budget long enough for one queued checklist command', () => { + expect(DEFAULT_DESCRIPTION_MUTATION_LOCK_TIMEOUT_MS).toBe(45_000); + }); + it('serializes concurrent work for the same provider and work item', async () => { const events: string[] = []; diff --git a/tests/unit/pm/jira/adapter.test.ts b/tests/unit/pm/jira/adapter.test.ts index 793724030..5592c061e 100644 --- a/tests/unit/pm/jira/adapter.test.ts +++ b/tests/unit/pm/jira/adapter.test.ts @@ -609,6 +609,67 @@ describe('JiraPMProvider', () => { expect(result.id).toMatch(/^inline-PROJ-1-[0-9a-f]{8}$/); }); + it('creates checklist with initial items in one ADF description write', async () => { + mockJiraClient.getIssue.mockResolvedValue({ + fields: { description: { type: 'doc', content: [] } }, + }); + mockAdfToPlainText.mockReturnValue('Existing.'); + const adfDoc = { type: 'doc', version: 1, content: [] }; + mockMarkdownToAdf.mockReturnValue(adfDoc); + mockJiraClient.updateIssue.mockResolvedValue(undefined); + + const result = await provider.createChecklistWithItems('PROJ-1', '✅ AC', [ + { name: 'First item' }, + { name: 'Done item', checked: true }, + ]); + + expect(mockMarkdownToAdf).toHaveBeenCalledTimes(1); + expect(mockMarkdownToAdf).toHaveBeenCalledWith( + 'Existing.\n\n### ✅ AC\n- [ ] First item\n- [x] Done item', + ); + expect(mockJiraClient.updateIssue).toHaveBeenCalledTimes(1); + expect(mockJiraClient.updateIssue).toHaveBeenCalledWith('PROJ-1', { description: adfDoc }); + expect(result).toMatchObject({ + name: '✅ AC', + workItemId: 'PROJ-1', + items: [ + { name: 'First item', complete: false }, + { name: 'Done item', complete: true }, + ], + }); + expect(result.items[0].id).toMatch(/^cl-[0-9a-f]{8}$/); + }); + + it('preserves concurrent bulk-created checklist sections', async () => { + let markdown = 'Existing.'; + mockJiraClient.getIssue.mockResolvedValue({ + fields: { description: { type: 'doc', content: [] } }, + }); + mockAdfToPlainText.mockImplementation(() => markdown); + mockMarkdownToAdf.mockImplementation((nextMarkdown) => nextMarkdown); + mockJiraClient.updateIssue.mockImplementation(async (_id, updates) => { + await sleep(5); + markdown = updates.description as string; + }); + + const results = await Promise.allSettled([ + provider.createChecklistWithItems('PROJ-1', '✅ Acceptance Criteria', [ + { name: 'Ready to ship' }, + ]), + provider.createChecklistWithItems('PROJ-1', '🔗 Dependencies', [ + { name: 'External API key' }, + { name: 'Vendor access', checked: true }, + ]), + ]); + + expect(results.every((result) => result.status === 'fulfilled')).toBe(true); + expect(markdown).toContain('### ✅ Acceptance Criteria'); + expect(markdown).toContain('- [ ] Ready to ship'); + expect(markdown).toContain('### 🔗 Dependencies'); + expect(markdown).toContain('- [ ] External API key'); + expect(markdown).toContain('- [x] Vendor access'); + }); + it('does NOT call createIssue (no subtask creation)', async () => { mockJiraClient.getIssue.mockResolvedValue({ fields: { description: { type: 'doc', content: [] } }, diff --git a/tests/unit/pm/linear/adapter.test.ts b/tests/unit/pm/linear/adapter.test.ts index cc3b7ef95..7bae68fd9 100644 --- a/tests/unit/pm/linear/adapter.test.ts +++ b/tests/unit/pm/linear/adapter.test.ts @@ -533,6 +533,32 @@ describe('LinearPMProvider', () => { expect(result.items).toEqual([]); }); + it('creates checklist with initial items in one description write', async () => { + mockIssueDescription('Existing.'); + + const result = await provider.createChecklistWithItems('issue-uuid', '✅ AC', [ + { name: 'First item', checked: false }, + { name: 'Done item', checked: true }, + ]); + + expect(mockUpdateIssue).toHaveBeenCalledTimes(1); + expect(mockUpdateIssue).toHaveBeenCalledWith( + 'issue-uuid', + expect.objectContaining({ + description: 'Existing.\n\n### ✅ AC\n- [ ] First item\n- [x] Done item', + }), + ); + expect(result).toMatchObject({ + name: '✅ AC', + workItemId: 'issue-uuid', + items: [ + { name: 'First item', complete: false }, + { name: 'Done item', complete: true }, + ], + }); + expect(result.items[0].id).toMatch(/^cl-[0-9a-f]{8}$/); + }); + it('waits for Linear read-after-write visibility before the next checklist append', async () => { let description = 'Existing.'; let staleDescription: string | null = null; @@ -592,6 +618,42 @@ describe('LinearPMProvider', () => { expect(description).toContain('### 🔗 Dependencies'); expect(description).toContain('- [ ] External API key'); }); + + it('preserves concurrent bulk-created checklist sections despite stale Linear reads', async () => { + let description = 'Existing.'; + let staleDescription: string | null = null; + mockGetIssue.mockImplementation(async () => { + if (staleDescription !== null) { + const value = staleDescription; + staleDescription = null; + return makeIssue({ description: value }); + } + return makeIssue({ description }); + }); + mockUpdateIssue.mockImplementation(async (_id, updates: { description?: string }) => { + await sleep(5); + staleDescription = description; + description = updates.description ?? description; + return makeIssue({ description }); + }); + + const results = await Promise.allSettled([ + provider.createChecklistWithItems('issue-uuid', '✅ Acceptance Criteria', [ + { name: 'Ready to ship' }, + ]), + provider.createChecklistWithItems('issue-uuid', '🔗 Dependencies', [ + { name: 'External API key' }, + { name: 'Vendor access', checked: true }, + ]), + ]); + + expect(results.every((result) => result.status === 'fulfilled')).toBe(true); + expect(description).toContain('### ✅ Acceptance Criteria'); + expect(description).toContain('- [ ] Ready to ship'); + expect(description).toContain('### 🔗 Dependencies'); + expect(description).toContain('- [ ] External API key'); + expect(description).toContain('- [x] Vendor access'); + }); }); describe('addChecklistItem (inline)', () => {