From 5a3d5a6f91d5229f6fcf1f6e4605be6f787b2b71 Mon Sep 17 00:00:00 2001 From: Igor Gerasimov Date: Wed, 22 Jul 2026 10:11:19 +0200 Subject: [PATCH] =?UTF-8?q?feat(consilium):=20ticket-first=20loops=20?= =?UTF-8?q?=E2=80=94=20auto=20commitPrefix,=20ticket=20in=20MR=20body,=20r?= =?UTF-8?q?equire-ticket=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The werush forge rejects any push whose commits lack a Jira issue key, so every develop round published nothing (branch intact locally, MR never opened) — and the ticket never reached the MR description. - Ticket intake (launchTicketReview) now sets the loop's commitPrefix to ': ' automatically (ReviewLaunchPlan.commitPrefix threaded to the factory) — every coder commit subject and the Draft-PR/MR title carry the key, so issue-key push policies pass. - The Draft-PR/MR BODY gains a ticket provenance line (key + link), threaded from the loop's launch provenance through SdlcHandoffRequest into buildPrStatusBody. Display-only, sanitized. - New config pipeline.consiliumLoop.requireTicketRef (default false): when on, a MANUAL launch without a ticket key is refused with an actionable 400 (the dialog's Jira field feeds commitPrefix). - Cleanup: the titleLabel control-strip regex carried RAW control bytes (rg treated the file as binary) — replaced with \u escapes. vitest: 180 passed across dispatch/route/executor/fsm suites. tsc clean. --- server/config/schema.ts | 8 +++++ server/routes/consilium-reviews.ts | 13 +++++++++ .../consilium/consilium-loop-controller.ts | 10 +++++++ server/services/consilium/trigger-dispatch.ts | 17 ++++++++++- server/services/sdlc/executor.ts | 20 +++++++++++++ .../consilium/consilium-reviews-route.test.ts | 29 +++++++++++++++++-- .../consilium/ticket-direct-dispatch.test.ts | 3 ++ 7 files changed, 96 insertions(+), 4 deletions(-) diff --git a/server/config/schema.ts b/server/config/schema.ts index 3ca64b84..6fadbfaa 100644 --- a/server/config/schema.ts +++ b/server/config/schema.ts @@ -336,6 +336,14 @@ export const ConfigSchema = z.object({ pollIntervalMs: z.coerce.number().int().min(1_000).max(60_000).default(5_000), /** Hard byte cap on the per-round unified diff (A2 bound). 1KiB..2MB. */ maxDiffBytes: z.coerce.number().int().min(1_024).max(2_000_000).default(200_000), + /** + * Ticket-first policy: when true, a MANUAL review launch (POST + * /api/consilium-reviews) must carry a ticket key (`commitPrefix`) — repos + * whose pre-receive hooks require an issue key in every commit would reject + * the publish of an unkeyed loop anyway. Ticket-intake loops always carry + * one automatically. Default false ⇒ byte-identical behaviour. + */ + requireTicketRef: z.boolean().default(false), /** * Fail-closed repo allowlist. Empty ⇒ no repo path is permitted (the * diff-context builder throws). config.yaml only — arrays are not diff --git a/server/routes/consilium-reviews.ts b/server/routes/consilium-reviews.ts index 725b008b..70c78568 100644 --- a/server/routes/consilium-reviews.ts +++ b/server/routes/consilium-reviews.ts @@ -131,6 +131,19 @@ export function registerConsiliumReviewRoutes(app: Express, deps: CreateConsiliu if (!req.projectId) return res.status(400).json({ error: "x-project-id header is required" }); const body = req.body as z.infer; + // Ticket-first policy (requireTicketRef): refuse an unkeyed manual launch — + // the team's forge rejects commits without an issue key, so an unkeyed loop + // could never publish its work anyway. Fail fast with an actionable 400. + if ( + deps.config?.().pipeline?.consiliumLoop?.requireTicketRef && + !sanitizeCommitPrefix(body.commitPrefix) + ) { + return res.status(400).json({ + error: + 'ticket key required: fill the Jira issue field (e.g. "PDO-123") — this project requires ticket-linked commits', + }); + } + try { const loop = await createConsiliumReview(deps, { projectId: req.projectId, diff --git a/server/services/consilium/consilium-loop-controller.ts b/server/services/consilium/consilium-loop-controller.ts index d12857cb..d2478ba9 100644 --- a/server/services/consilium/consilium-loop-controller.ts +++ b/server/services/consilium/consilium-loop-controller.ts @@ -2685,6 +2685,16 @@ export class ConsiliumLoopController { // OPTIONAL per-loop prefix (e.g. a Jira key), threaded to every SDLC-coder // git subject line + the Merge-Request title. commitPrefix: loop.commitPrefix ?? undefined, + // Ticket-first provenance for the Draft-PR/MR body (inert display): the + // intake ticket recorded at launch, when the loop came from one. + ticketRef: (() => { + const src = loop.triggerProvenance?.spec?.source; + if (!src || typeof src.ref !== "string" || src.ref.length === 0) return undefined; + return { + key: src.ref, + ...(typeof src.url === "string" ? { url: src.url } : {}), + }; + })(), actionPoints: routedActionPoints, allowedRepoPaths: cfg.allowedRepoPaths, ownerId: loop.createdBy ?? "", diff --git a/server/services/consilium/trigger-dispatch.ts b/server/services/consilium/trigger-dispatch.ts index 7bd544c4..a2540f09 100644 --- a/server/services/consilium/trigger-dispatch.ts +++ b/server/services/consilium/trigger-dispatch.ts @@ -640,10 +640,15 @@ export async function launchTicketReview( // UNTRUSTED title → single-line control-strip + clamp for the inert passport label. const titleLabel = args.ticket.title // eslint-disable-next-line no-control-regex - .replace(/[-]+/g, " ") + .replace(/[\u0000-\u001f\u007f]+/g, " ") .trim() .slice(0, 120); const anchor = `ticket:${args.ticket.kind}:${args.ticket.key}`; + // Ticket-first commits: the ticket KEY becomes the loop's commitPrefix so every + // coder commit subject and the Draft-PR/MR title carry it — a push to a repo + // whose pre-receive hook requires an issue key can land. The key is already + // connector-sanitised; the extra filter is defence-in-depth (option-safe charset). + const safeKey = args.ticket.key.replace(/[^A-Za-z0-9_-]/g, "").slice(0, 40); return launchReviewWithDedup(deps, trigger, { projectId: args.projectId, @@ -651,6 +656,7 @@ export async function launchTicketReview( preset: args.preset ?? SPEC_DEFAULT_PRESET, // ADR-004: review the fresh remote default (poller-resolved), not the tree. ref: args.ref ?? null, + ...(safeKey.length > 0 ? { commitPrefix: `${safeKey}: ` } : {}), engineerInstruction, // Per-ticket dedup: the synthetic anchor rides the spec-dedup seam, so two // tickets targeting one repo each fire their own loop (mirrors per-spec dedup). @@ -687,6 +693,13 @@ export interface ReviewLaunchPlan { engineerInstruction?: string; objectiveExtra?: string; eventSummary?: string; + /** + * Ticket-first commits (ADR-004): per-loop commit-message/MR-title prefix + * (e.g. `"PDO-922: "`), persisted on the loop and applied by the SDLC executor + * to every commit subject + the Draft-PR/MR title. Absent ⇒ no prefix + * (byte-identical for every non-ticket path). Caller-sanitised. + */ + commitPrefix?: string; /** * SPEC-1: operator/spec-selected skill ids, resolved PROJECT-SCOPED inside the * factory (a foreign id throws → "failed", caught by T4). Absent for every @@ -848,6 +861,8 @@ export async function launchReviewWithDedup( // Exactly one of these is set (engineerInstruction wins in the factory). engineerInstruction: plan.engineerInstruction, objectiveExtra: plan.objectiveExtra, + // Ticket-first commits: the caller-sanitised ticket prefix (absent ⇒ none). + commitPrefix: plan.commitPrefix, // §6: record which trigger + event (+ role) started the loop. triggerProvenance: provenance, }); diff --git a/server/services/sdlc/executor.ts b/server/services/sdlc/executor.ts index c02a0efa..355c8d88 100644 --- a/server/services/sdlc/executor.ts +++ b/server/services/sdlc/executor.ts @@ -329,6 +329,12 @@ export interface SdlcHandoffRequest { * the create route; re-sanitized defensively at each call site below. */ commitPrefix?: string; + /** + * The intake ticket this loop implements (from the loop's launch provenance), + * surfaced in the Draft-PR/MR BODY so the reviewer lands on the requirement in + * one click. Display-only inert text/link; absent ⇒ no ticket line. + */ + ticketRef?: { key: string; url?: string }; /** The round's open action points to implement (one coder run EACH). */ actionPoints: readonly ActionPoint[]; /** Fail-closed repo allowlist (H-5). */ @@ -757,6 +763,8 @@ export interface PrStatusBodyInput { * pre-Stage-A body). */ finalVerification?: FinalVerification; + /** The intake ticket (key + optional URL) — one provenance line in the header. */ + ticketRef?: { key: string; url?: string }; } /** @@ -781,6 +789,17 @@ export function buildPrStatusBody(input: PrStatusBodyInput): string { `- Loop: \`${sanitizeLine(loopId, 80)}\``, `- Round: ${round}`, `- Repo: \`${sanitizeLine(repoName, 120)}\``, + // Ticket-first provenance: the requirement this round implements, when known. + // Key sanitized single-line; the URL only renders for a well-formed http(s) link. + ...(input.ticketRef?.key + ? [ + `- Ticket: ${sanitizeLine(input.ticketRef.key, 60)}${ + input.ticketRef.url && /^https?:\/\//.test(input.ticketRef.url) + ? ` — ${sanitizeLine(input.ticketRef.url, 300)}` + : "" + }`, + ] + : []), ]; // Action points addressed — from the verdict (UNTRUSTED -> sanitize + clamp). @@ -2427,6 +2446,7 @@ async function pushAndOpenPr( actionPoints: req.actionPoints, outcomes, finalVerification, + ticketRef: req.ticketRef, }), }); if (!pr.ok) { diff --git a/tests/unit/consilium/consilium-reviews-route.test.ts b/tests/unit/consilium/consilium-reviews-route.test.ts index 7c3350c4..aa966da1 100644 --- a/tests/unit/consilium/consilium-reviews-route.test.ts +++ b/tests/unit/consilium/consilium-reviews-route.test.ts @@ -25,7 +25,7 @@ import { createConsiliumReview } from "../../../server/services/consilium/review const mockedCreate = vi.mocked(createConsiliumReview); -function makeApp() { +function makeApp(deps: unknown = {}) { const app = express(); app.use(express.json()); // Stand in for requireAuth + requireProject (applied at mount in routes.ts). @@ -34,11 +34,16 @@ function makeApp() { (req as unknown as { projectId: string }).projectId = "project-1"; next(); }); - // deps are unused — the factory is fully mocked. - registerConsiliumReviewRoutes(app, {} as never); + // deps default {} — the factory is fully mocked; ticket-first tests pass a config. + registerConsiliumReviewRoutes(app, deps as never); return app; } +/** Deps carrying ONLY the ticket-first flag (everything else mocked/unused). */ +const ticketFirstDeps = { + config: () => ({ pipeline: { consiliumLoop: { requireTicketRef: true } } }), +}; + const VALID_BODY = { repoPath: "/repos/widget", preset: "sdlc-cross-review" as const }; describe("POST /api/consilium-reviews — distinct 400s for the two confinement boundaries (MED-3)", () => { @@ -82,6 +87,24 @@ describe("POST /api/consilium-reviews — distinct 400s for the two confinement expect(allowlistRes.body.error).not.toBe(workspaceRes.body.error); }); + it("TICKET-FIRST: requireTicketRef + no commitPrefix → 400 with the ticket message, factory never called", async () => { + const res = await request(makeApp(ticketFirstDeps)) + .post("/api/consilium-reviews") + .send(VALID_BODY); + expect(res.status).toBe(400); + expect(String(res.body.error)).toContain("ticket key required"); + expect(mockedCreate).not.toHaveBeenCalled(); + }); + + it("TICKET-FIRST: requireTicketRef + a commitPrefix → passes the gate to the factory", async () => { + mockedCreate.mockResolvedValueOnce({ id: "loop-1" } as never); + const res = await request(makeApp(ticketFirstDeps)) + .post("/api/consilium-reviews") + .send({ ...VALID_BODY, commitPrefix: "PDO-922: " }); + expect(res.status).toBe(201); + expect(mockedCreate).toHaveBeenCalledTimes(1); + }); + it("a successful create returns 201 with the loop row", async () => { mockedCreate.mockResolvedValueOnce({ id: "loop-1", status: "PENDING" } as never); const res = await request(makeApp()).post("/api/consilium-reviews").send(VALID_BODY); diff --git a/tests/unit/consilium/ticket-direct-dispatch.test.ts b/tests/unit/consilium/ticket-direct-dispatch.test.ts index 4162c43d..feb41470 100644 --- a/tests/unit/consilium/ticket-direct-dispatch.test.ts +++ b/tests/unit/consilium/ticket-direct-dispatch.test.ts @@ -79,6 +79,9 @@ describe("launchTicketReview (ADR-004 Block A)", () => { expect(plan.preset).toBe("sdlc-cross-review"); // No poller-resolved ref ⇒ working-tree HEAD (null), the pre-existing default. expect(plan.ref).toBeNull(); + // Ticket-first commits: the ticket key rides as the loop's commit prefix so + // coder commits/MR titles pass issue-key push policies. + expect(plan.commitPrefix).toBe("PDO-850: "); // DoD-first: criteria reach the objective BEFORE the body (H1 clamp discipline). const instruction = plan.engineerInstruction as string;