Skip to content

Commit 002079d

Browse files
authored
fix(workflows): repo-bound task without workflow_ref → coding/new-task-v1, not default/agent-v1 (#594)
* fix(workflows): repo-bound task w/o workflow_ref → coding/new-task-v1, not default/agent-v1 #296 (workflow-driven tasks) replaced the task_type→workflow mapping with a resolution ladder, but left the repo-aware rung ('Phase 4') unwired. Every task with no explicit workflow_ref — all Slack tasks and all #247 orchestration children (neither sets one) — fell through to the platform default default/agent-v1: the freeform, repo-less agent prompt with NO git/PR discipline. The agent then improvised (gh api / gh pr create against an empty local clone), so ensure_pr found no commits and recorded pr_url=null, screenshot→Linear routing lost its branch signal, and #247 A4 stacking broke (children couldn't fetch the unpushed predecessor branch). Re-wire the missing rung minimally: resolveWorkflowRef takes hasRepo; an absent ref with a repo present resolves to coding/new-task-v1 (the disciplined coding workflow — edit locally, commit, push, platform opens the PR via ensure_pr), matching pre-#296 behaviour. Repo-less tasks still default to default/agent-v1. An explicit workflow_ref always wins. The single create-task-core call site passes Boolean(body.repo), so both Slack and orchestration (which create via createTaskCore with repo set) inherit the fix. Upstream regression — worth an upstream issue too. Updated workflows + create-task-core tests; the old test asserting default/agent-v1 for a repo task encoded the regression. (cherry picked from commit 99b5a17) (cherry picked from commit 9d8b25d) * refactor: pin coding workflow at channel call sites, not the resolver default Rework per review (#594): follow the merged #547 precedent instead of changing the shared resolver default. A repo-bound channel task must run the disciplined coding workflow (clone → commit → push → platform opens the PR), not the repo-less default/agent-v1 that records pr_url=null — but that "repo task ⇒ coding workflow" decision now lives explicitly at each channel's createTaskCore call site rather than as an implicit resolver-level inference. Addresses the review point-by-point: - B3(a): revert resolveWorkflowRef to its single-arg form (no hasRepo); pin workflow_ref: CODING_WORKFLOW_ID in the Linear and Slack processors, mirroring the Jira processor (#546/#547). One visible decision per channel, uniform across all three. (No #247-child path exists on main.) - B2: moot — no resolver behaviour change, so WORKFLOWS.md / API_CONTRACT.md / USER_GUIDE.md all stay accurate. No doc edits, no mirror regen needed. - N1: moot — the hasRepo param is gone, so the "forgot hasRepo" footgun can't exist. - N2: promote the 'coding/new-task-v1' literal to CODING_WORKFLOW_ID next to DEFAULT_WORKFLOW_ID, with a module-load invariant asserting both ids exist in DESCRIPTORS (a descriptor rename now fails at import, not as a runtime TypeError). Jira's literal switched to the constant too — uniform. - N3: moot — the stale "last rung" JSDoc was in the reverted resolver block. - §6 test gap: create-task-core caller tests now assert the real behaviour (omitted ref → default/agent-v1, with AND without a repo — the symmetric repo-less case that was missing). Channel processor tests (Linear/Slack/Jira) assert each pins workflow_ref: 'coding/new-task-v1'. Full cdk build green (compile + jest + eslint + synth).
1 parent f765219 commit 002079d

8 files changed

Lines changed: 90 additions & 5 deletions

cdk/src/handlers/jira-webhook-processor.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { reportIssueFailure } from './shared/jira-feedback';
2525
import { resolveJiraOauthToken } from './shared/jira-oauth-resolver';
2626
import { logger } from './shared/logger';
2727
import type { Attachment } from './shared/types';
28+
import { CODING_WORKFLOW_ID } from './shared/workflows';
2829

2930
const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({}));
3031

@@ -370,7 +371,7 @@ export async function handler(event: ProcessorEvent): Promise<void> {
370371
// Explicit coding workflow: a label-triggered Jira task always targets a
371372
// mapped repo, so it must not fall through the resolution ladder to the
372373
// repo-less default/agent-v1 (which never commits or opens a PR). #546
373-
workflow_ref: 'coding/new-task-v1',
374+
workflow_ref: CODING_WORKFLOW_ID,
374375
...(attachments.length > 0 && { attachments }),
375376
},
376377
{

cdk/src/handlers/linear-webhook-processor.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { reportIssueFailure } from './shared/linear-feedback';
2525
import { resolveLinearOauthToken } from './shared/linear-oauth-resolver';
2626
import { logger } from './shared/logger';
2727
import type { Attachment } from './shared/types';
28+
import { CODING_WORKFLOW_ID } from './shared/workflows';
2829

2930
const ddb = DynamoDBDocumentClient.from(new DynamoDBClient({}));
3031

@@ -300,6 +301,11 @@ export async function handler(event: ProcessorEvent): Promise<void> {
300301
{
301302
repo,
302303
task_description: taskDescription,
304+
// Explicit coding workflow: a label-triggered Linear task always targets a
305+
// mapped repo, so it must not fall through the resolution ladder to the
306+
// repo-less default/agent-v1 (which never commits or opens a PR). Mirrors
307+
// the Jira processor (#546/#547). See CODING_WORKFLOW_ID.
308+
workflow_ref: CODING_WORKFLOW_ID,
303309
...(attachments.length > 0 && { attachments }),
304310
},
305311
{

cdk/src/handlers/shared/workflows.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,32 @@ const DESCRIPTORS: Record<string, WorkflowDescriptor> = {
148148
/** The platform default workflow — the last rung of the resolution ladder. */
149149
export const DEFAULT_WORKFLOW_ID = 'default/agent-v1';
150150

151+
/**
152+
* The disciplined coding workflow (clone → edit → commit → push → platform
153+
* opens the PR via ``ensure_pr``). A repo-bound task with no explicit
154+
* ``workflow_ref`` must run THIS, not the repo-less {@link DEFAULT_WORKFLOW_ID}
155+
* (which improvises against an empty local clone and records ``pr_url=null``).
156+
*
157+
* The channel processors (Linear/Jira/Slack) and the #247 orchestration release
158+
* path pin this explicitly at their ``createTaskCore`` call sites — see
159+
* ``jira-webhook-processor.ts`` (#546/#547, the first instance) — rather than
160+
* relying on a resolver-level default, so the "repo task ⇒ coding workflow"
161+
* decision lives at one visible place per channel and stays uniform across them.
162+
*/
163+
export const CODING_WORKFLOW_ID = 'coding/new-task-v1';
164+
165+
// Fail at MODULE LOAD (deploy/import time), not at request time, if either
166+
// well-known id ever drifts out of DESCRIPTORS — e.g. a descriptor rename.
167+
// Without this a rename surfaces as an obscure runtime
168+
// ``TypeError: Cannot read properties of undefined (reading 'id')`` inside the
169+
// Lambda on the first affected task; TypeScript's ``Record<string, …>`` index
170+
// type will not catch it (N2).
171+
for (const requiredId of [DEFAULT_WORKFLOW_ID, CODING_WORKFLOW_ID]) {
172+
if (!DESCRIPTORS[requiredId]) {
173+
throw new Error(`Workflow descriptor "${requiredId}" is missing from DESCRIPTORS`);
174+
}
175+
}
176+
151177
/** Pattern for a valid workflow ref: ``<domain>/<name>-vN[@<constraint>]``. */
152178
const WORKFLOW_REF_PATTERN = /^[a-z][a-z0-9-]*\/[a-z][a-z0-9-]*-v\d+(@[^\s]+)?$/;
153179

@@ -186,7 +212,16 @@ export type WorkflowResolutionError = 'unknown_id' | 'unsatisfiable_version';
186212
* resolution ladder (WORKFLOWS.md §"Replacing task types"):
187213
* 1. explicit ``workflow_ref`` (id + optional ``@constraint``);
188214
* 2. (Blueprint default — Phase 4, not yet wired);
189-
* 3. the platform default ``default/agent-v1``.
215+
* 3. the repo-less platform default ``default/agent-v1``.
216+
*
217+
* NOTE: an absent ref falls back to the repo-less default here regardless of
218+
* whether the request carries a repo. A repo-bound channel task (Linear/Jira/
219+
* Slack/#247) must NOT rely on this fallback — it pins an explicit
220+
* ``workflow_ref: CODING_WORKFLOW_ID`` at its call site so a repo task runs the
221+
* disciplined coding workflow (commit + push + PR), not the repo-less agent
222+
* that records ``pr_url=null``. That decision lives at the call site, one per
223+
* channel, mirroring ``jira-webhook-processor.ts`` (#546/#547) — see
224+
* {@link CODING_WORKFLOW_ID}.
190225
*
191226
* Returns ``null`` when an explicit ref cannot be resolved — either the id is
192227
* unknown OR an ``@constraint`` pins a version the platform does not ship. The

cdk/src/handlers/slack-command-processor.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { logger } from './shared/logger';
2525
import { slackFetch } from './shared/slack-api';
2626
import { getSlackSecret, SLACK_SECRET_PREFIX } from './shared/slack-verify';
2727
import type { Attachment } from './shared/types';
28+
import { CODING_WORKFLOW_ID } from './shared/workflows';
2829
import type { SlackCommandPayload } from './slack-commands';
2930

3031
/**
@@ -243,6 +244,11 @@ async function handleSubmit(event: MentionEvent, args: string[], reply: ReplyFn)
243244
repo,
244245
issue_number: issueNumber,
245246
task_description: description,
247+
// Explicit coding workflow: a Slack-submitted task targets a repo, so it
248+
// must not fall through the resolution ladder to the repo-less
249+
// default/agent-v1 (which never commits or opens a PR). Mirrors the Jira
250+
// processor (#546/#547). See CODING_WORKFLOW_ID.
251+
workflow_ref: CODING_WORKFLOW_ID,
246252
...(attachments.length > 0 && { attachments }),
247253
},
248254
{

cdk/test/handlers/linear-webhook-processor.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ describe('linear-webhook-processor handler', () => {
176176
expect(reqBody.repo).toBe('org/repo');
177177
expect(reqBody.task_description).toContain('ABC-42: Fix the login bug');
178178
expect(reqBody.task_description).toContain('Users cannot log in.');
179+
// Must pin the coding workflow — an absent workflow_ref falls through the
180+
// resolution ladder to default/agent-v1, which never opens a PR. Mirrors
181+
// the Jira processor (#546/#547).
182+
expect(reqBody.workflow_ref).toBe('coding/new-task-v1');
179183
expect(ctx.userId).toBe('cognito-user-1');
180184
expect(ctx.channelSource).toBe('linear');
181185
expect(ctx.channelMetadata).toMatchObject({

cdk/test/handlers/shared/create-task-core.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,15 +589,34 @@ describe('createTaskCore', () => {
589589
expect(result.statusCode).toBe(201);
590590
});
591591

592-
test('resolves the default workflow when workflow_ref is omitted', async () => {
592+
test('resolves the platform default when workflow_ref is omitted (repo present)', async () => {
593+
// createTaskCore does NOT infer the coding workflow from the mere presence
594+
// of a repo — that "repo task ⇒ coding workflow" decision is pinned by each
595+
// CHANNEL processor at its call site (Linear/Jira/Slack pass
596+
// workflow_ref: CODING_WORKFLOW_ID; see those processors' tests). A raw
597+
// createTaskCore call with no ref falls through the resolution ladder to the
598+
// repo-less platform default, whether or not a repo is attached.
593599
const result = await createTaskCore(
594600
{ repo: 'org/repo', task_description: 'Fix the bug' },
595601
makeContext(),
596602
'req-default',
597603
);
598604
expect(result.statusCode).toBe(201);
599605
const body = JSON.parse(result.body);
600-
// No workflow_ref ⇒ the resolution ladder falls to the platform default.
606+
expect(body.data.resolved_workflow).toEqual({ id: 'default/agent-v1', version: '1.0.0' });
607+
});
608+
609+
test('resolves the platform default when workflow_ref is omitted AND no repo (symmetric)', async () => {
610+
// §6 symmetric coverage: the repo-less path through the real caller. Guards
611+
// against a future fallback-branch inversion that a repo-pinned test would
612+
// miss (the repo-less default must stay default/agent-v1).
613+
const result = await createTaskCore(
614+
{ task_description: 'Do some research' },
615+
makeContext(),
616+
'req-default-norepo',
617+
);
618+
expect(result.statusCode).toBe(201);
619+
const body = JSON.parse(result.body);
601620
expect(body.data.resolved_workflow).toEqual({ id: 'default/agent-v1', version: '1.0.0' });
602621
});
603622

cdk/test/handlers/shared/workflows.test.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import * as fs from 'fs';
2121
import * as path from 'path';
2222
import * as yaml from 'js-yaml';
2323
import {
24+
CODING_WORKFLOW_ID,
2425
DEFAULT_WORKFLOW_ID,
2526
WORKFLOW_MODEL_ALLOWLIST,
2627
disallowedWorkflowModel,
@@ -72,12 +73,21 @@ describe('resolveWorkflowRef', () => {
7273
expect(resolveWorkflowRefError(undefined)).toBeNull();
7374
});
7475

75-
test('falls back to the platform default when ref is absent', () => {
76+
test('falls back to the repo-less platform default when ref is absent', () => {
7677
expect(resolveWorkflowRef(undefined)).toEqual({ id: DEFAULT_WORKFLOW_ID, version: '1.0.0' });
7778
expect(resolveWorkflowRef(null)).toEqual({ id: DEFAULT_WORKFLOW_ID, version: '1.0.0' });
7879
expect(resolveWorkflowRef('')).toEqual({ id: DEFAULT_WORKFLOW_ID, version: '1.0.0' });
7980
});
8081

82+
test('CODING_WORKFLOW_ID resolves to the disciplined coding workflow', () => {
83+
// The channel processors pin this at the call site for a repo-bound task
84+
// (the "repo task ⇒ coding workflow" decision lives per-channel, not in the
85+
// resolver default). Assert the constant points at a real, repo-bound,
86+
// non-read-only workflow so a descriptor rename can't silently mispoint it.
87+
const resolved = resolveWorkflowRef(CODING_WORKFLOW_ID);
88+
expect(resolved).toEqual({ id: 'coding/new-task-v1', version: '1.0.0' });
89+
});
90+
8191
test('returns null for an unknown but well-formed ref', () => {
8292
expect(resolveWorkflowRef('coding/does-not-exist-v1')).toBeNull();
8393
});

cdk/test/handlers/slack-command-processor.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ describe('slack-command-processor handler', () => {
170170
expect(reqBody.repo).toBe('org/repo');
171171
expect(reqBody.issue_number).toBe(42);
172172
expect(reqBody.task_description).toBe('add validation');
173+
// Must pin the coding workflow — an absent workflow_ref falls through the
174+
// resolution ladder to default/agent-v1, which never opens a PR. Mirrors
175+
// the Jira processor (#546/#547).
176+
expect(reqBody.workflow_ref).toBe('coding/new-task-v1');
173177
expect(ctx.channelSource).toBe('slack');
174178
expect(ctx.userId).toBe('cognito-1');
175179
// mention_thread_ts flows to channel_metadata

0 commit comments

Comments
 (0)