diff --git a/CHANGELOG.md b/CHANGELOG.md index 57416ad13..3ef512989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ All notable user-visible changes to CASCADE are documented here. The format is l ### Fixed +- **`ReadWorkItem` examples now render PM IDs as runnable bare CLI values.** Native-tool prompt guidance and `cascade-tools pm read-work-item --help` now show `--workItemId abc123` instead of JSON-string-literal forms like `--workItemId '"abc123"'`. The CLI also strips one accidental outer quote layer for `ReadWorkItem` IDs only, so a copied bad example no longer sends literal quote characters to the PM provider. See Trello card [M5f9T1D7](https://trello.com/c/M5f9T1D7/673-frictionlow-readworkitem-example-quoting-produced-trello-card-id-with-literal-quotes). + +- **`cascade-tools scm create-pr-review` now accepts `--body-file ` and `--body-file -`.** This matches the generated CreatePRReview guidance and the existing CreatePR / PostPRComment file-input pattern for long Markdown bodies. See Trello card [7kmo42o6](https://trello.com/c/7kmo42o6/691-friction-tooling-low-createprreview-docs-advertise-body-file-but-cli-rejects-it). + +- **PM `cascade-tools` runtime failures now exit non-zero with structured failure envelopes.** PM core commands now throw on fatal provider/API failures, letting `createCLICommand()` emit `{"success":false,"error":{"type":"runtime","message":"..."}}` instead of wrapping prose like `Error posting comment: ...` inside `success:true` data. Intentional non-fatal PM outcomes such as guarded move no-ops and friction retry queueing remain successful command results. See Trello card [lU9mHLJT](https://trello.com/c/lU9mHLJT/686-friction-tooling-low-pm-post-comment-returned-success-envelope-with-embedded-400-error). + +- **Native-tool prompt examples for enum, scalar, number, and primitive-array flags now render as runnable CLI syntax instead of JSON string literals.** Agent-facing guidance now shows forms such as `cascade-tools scm create-pr-review --event APPROVE` and repeatable primitive arrays as `--labels bug --labels docs`, while object and array-of-object flags continue to render shell-quoted JSON payloads. See Trello card [l9Sira7y](https://trello.com/c/l9Sira7y/685-friction-tooling-low-create-pr-review-docs-show-quoted-enum-but-cli-requires-raw-enum). + - **`resolve-conflicts` agent no longer silently skips when GitHub's async mergeability computation hasn't resolved by the time the `pull_request` webhook is processed** (spec 020). `PRConflictDetectedTrigger` previously exhausted a 2×2s synchronous retry budget and silently discarded the event when `mergeable === null` — because GitHub never sends a follow-up webhook once mergeability resolves, the `resolve-conflicts` agent never fired. The trigger now returns `TriggerResult.deferredRecheck`, which causes the router to schedule a bare BullMQ delayed re-check job ~45s later via `scheduleCoalescedJob` (deduped per PR). The worker re-dispatches via the trigger registry to get fresh mergeability state. Multiple rapid webhooks for the same PR coalesce to a single re-check job. If mergeability is still `null` after the re-check fires, a Sentry event is captured under tag `mergeability_recheck_exhausted` and a WARN log is emitted — not a silent discard. Observed live on ucho/PR #329 (2026-05-07). See [spec 020](docs/specs/020-github-mergeability-deferred-recheck.md). ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 6134725f0..51d4ff117 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -25,7 +25,7 @@ Flow: `PM/SCM/alerting webhook → Router → Redis → Worker → TriggerRegist **Capacity-gate invariant.** Every PM router adapter (`src/router/adapters/{linear,trello,jira}.ts`) must wrap `triggerRegistry.dispatch(ctx)` in PM-provider `AsyncLocalStorage` scope via the shared `withPMScopeForDispatch(fullProject, dispatch)` helper at `src/router/adapters/_shared.ts` — in addition to the per-PM-type credential scope (`withLinearCredentials` / `withTrelloCredentials` / `withJiraCredentials`). Without the PM-provider wrapping, the pipeline-capacity gate at `src/triggers/shared/pipeline-capacity-gate.ts` cannot resolve `getPMProvider()`, **fails closed** under the spec-017 fail-closed policy (blocks the run + ERROR + Sentry capture under tag `pipeline_capacity_gate_no_pm_provider`), and `maxInFlightItems` is silently disabled for the PM-source path. Mirror the GitHub adapter's existing correct shape at `src/router/adapters/github.ts:dispatchWithCredentials`. The static guard at `tests/unit/integrations/pm-router-adapter-pm-scope.test.ts` enforces this at CI time — adding a new PM router adapter without the wrapping fails CI with a precise file path. -Integration abstraction lives in `src/integrations/`. For **adding a new PM provider**, see @src/integrations/README.md — PM providers (Trello, JIRA, Linear) use the `PMProviderManifest` registry with a **behavioral conformance harness** (spec 009 — config round-trip, discovery shape, full lifecycle scenario, auth-header provenance, single-entrypoint invariant). Each provider owns its Zod config schema (`src/integrations/pm//config-schema.ts`) as the single source of truth — the central `src/config/schema.ts` imports it. PM adapter method signatures use branded `StateId` / `LabelId` / `ContainerId` from `src/pm/ids.ts` to make state-name-vs-ID confusion a compile error at direct-adapter call sites. All runtime surfaces (router, worker, CLI, dashboard) register integrations through a single entrypoint at `src/integrations/entrypoint.ts`. **Spec 010 follow-ups** added generic `pm.discovery.createLabel` / `createCustomField` mutation endpoints + `currentUser` discovery capability + real shared React components for every `StandardStepKind` under `web/src/components/projects/pm-providers/steps/`. **Spec 011** migrated all three production providers (Trello, JIRA, Linear) onto those shared components, added a 7th `StandardStepKind: custom-field-mapping`, widened `container-pick` / `project-scope` / `webhook-url-display` with optional props, and deleted the three legacy `pm-wizard-{trello,jira,linear}-steps.tsx` files. **Spec 012** migrated each provider's webhook UX (programmatic create for Trello/JIRA, signing-secret + instructions for Linear) into per-provider manifest webhook adapters (Fragment compositions around the shared `WebhookUrlDisplayStep`); deleted the legacy `WebhookStep` + `LinearWebhookInfoPanel` + `useWebhookManagement` + `useLinearWebhookInfo`. Every PM wizard step now renders via the manifest path without exception. A new PM provider writes zero edits to shared orchestration (`pm-wizard.tsx`, `pm-wizard-common-steps.tsx`, `pm-wizard-hooks.ts`); provider-specific UI ships either as `kind: 'custom'` steps or as Fragment compositions inside the provider folder's wizard adapters. SCM (GitHub) and alerting (Sentry) still use the legacy `IntegrationModule` pattern via self-registration in `src/github/register.ts` + `src/sentry/register.ts`. Don't improvise; the README covers both patterns. +Integration abstraction lives in `src/integrations/`. For **adding a new PM provider**, see @src/integrations/README.md — PM providers (Trello, JIRA, Linear) use the `PMProviderManifest` registry with a **behavioral conformance harness** (spec 009 — config round-trip, discovery shape, full lifecycle scenario, auth-header provenance, single-entrypoint invariant). Each provider owns its Zod config schema (`src/integrations/pm//config-schema.ts`) as the single source of truth — the central `src/config/schema.ts` imports it. PM adapter method signatures use branded `StateId` / `LabelId` / `ContainerId` from `src/pm/ids.ts` to make state-name-vs-ID confusion a compile error at direct-adapter call sites. All runtime surfaces (router, worker, CLI, dashboard) register integrations through a single entrypoint at `src/integrations/entrypoint.ts`. **Spec 010 follow-ups** added generic `pm.discovery.createLabel` / `createCustomField` mutation endpoints + `currentUser` discovery capability + real shared React components for every `StandardStepKind` under `web/src/components/projects/pm-providers/steps/`. **Spec 011** migrated all three production providers (Trello, JIRA, Linear) onto those shared components, added a 7th `StandardStepKind: custom-field-mapping`, widened `container-pick` / `project-scope` / `webhook-url-display` with optional props, and deleted the three legacy `pm-wizard-{trello,jira,linear}-steps.tsx` files. **Spec 012** migrated each provider's webhook UX (programmatic create for Trello/JIRA, signing-secret + instructions for Linear) into per-provider manifest webhook adapters (Fragment compositions around the shared `WebhookUrlDisplayStep`); deleted the legacy `WebhookStep` + `LinearWebhookInfoPanel` + `useWebhookManagement` + `useLinearWebhookInfo`. Every PM wizard step now renders via the manifest path without exception. A new PM provider writes one import in the backend barrel (`src/integrations/pm/index.ts`) and one import in the frontend barrel (`web/src/components/projects/pm-providers/index.ts`); `pm-wizard.tsx`, `pm-wizard-common-steps.tsx`, and `pm-wizard-hooks.ts` receive zero edits. The verification-button readiness path (`areCredentialsReadyFromMetadata` in `pm-wizard-hooks.ts`) and the mutation auth path (`buildProviderAuthArgFromMetadata`) are metadata-driven and require no changes for a new provider. **The shared dashboard state (`pm-wizard-state.ts`) does still require edits**: new providers must add their credential fields to `WizardState` (e.g. `asanaApiKey: string`) and the corresponding action types to `WizardAction`; config-shape hydration belongs on the provider's `ProviderWizardDefinition.buildEditState` — see step 4 of @src/integrations/README.md. Provider-specific hooks, auth metadata, verification display formatting, and UI live inside the provider folder (`kind: 'custom'` steps or Fragment compositions around shared steps). SCM (GitHub) and alerting (Sentry) still use the legacy `IntegrationModule` pattern via self-registration in `src/github/register.ts` + `src/sentry/register.ts`. Don't improvise; the README covers both patterns. ## PR checkout (worker) — gotcha diff --git a/docs/architecture/06-integration-layer.md b/docs/architecture/06-integration-layer.md index a6477a57f..d4ef73a73 100644 --- a/docs/architecture/06-integration-layer.md +++ b/docs/architecture/06-integration-layer.md @@ -56,6 +56,8 @@ const integrationRegistry: IntegrationRegistry; // singleton The PM barrel (`src/integrations/pm/index.ts`) imports each provider once, then mirrors each manifest's `pmIntegration` into `integrationRegistry`. New PM providers add one provider folder plus one import in that barrel; shared router, worker, dashboard, CLI, and config files are guarded against provider-specific edits by conformance tests. +The dashboard has a matching frontend PM-provider boundary under `web/src/components/projects/pm-providers//`. A provider owns its wizard definition, auth metadata, credential persistence metadata, save config serialization, edit-mode hydration, discovery/mutation hooks, webhook UX composition, and provider-specific state slice in that folder. The shared wizard files (`pm-wizard.tsx`, `pm-wizard-hooks.ts`, and `pm-wizard-common-steps.tsx`) only render registered provider definitions and run metadata-driven verification/save helpers, so adding a provider does not require editing them. New providers add one frontend barrel import in `web/src/components/projects/pm-providers/index.ts`; `pm-wizard-state.ts` is the remaining explicit shared-dashboard exception while it composes provider state slices into the aggregate `WizardState` and reducer. + `src/pm/integration.ts` — extends `IntegrationModule` with PM-specific methods: - `createProvider(project)` — create a `PMProvider` instance for CRUD operations diff --git a/docs/architecture/07-gadgets.md b/docs/architecture/07-gadgets.md index 21dedd7ad..0225854f3 100644 --- a/docs/architecture/07-gadgets.md +++ b/docs/architecture/07-gadgets.md @@ -136,6 +136,8 @@ The `cascade-tools` binary uses a separate oclif config (`bin/cascade-tools.js`) New domain commands should not add branches in these helpers. They declare behavior through their `ToolDefinition` metadata (`cliAliases`, examples, file input alternatives, auto-resolution), and the shared generators consume it. +Core functions passed to `createCLICommand()` own domain work only. On fatal runtime/API/provider failures they throw, and the shared factory converts that exception into the structured `{"success":false,"error":{"type":"runtime","message":"..."}}` stdout envelope plus exit code 1. A returned value is always serialized as successful `data`, so gadgets must not return sentinel error strings such as `Error reading work item: ...` for fatal failures. Non-fatal command states that are part of the contract, such as guarded PM move no-ops or friction retry queueing, remain successful returns. + ## Session State `src/gadgets/sessionState.ts` diff --git a/src/backends/shared/nativeToolPrompts.ts b/src/backends/shared/nativeToolPrompts.ts index 0fd4fe7f8..f56d75ab3 100644 --- a/src/backends/shared/nativeToolPrompts.ts +++ b/src/backends/shared/nativeToolPrompts.ts @@ -1,3 +1,4 @@ +import { formatJsonExample, formatShellScalar } from '../../gadgets/shared/cli/shellValues.js'; import type { ContextInjection, ToolManifest } from '../types.js'; import { buildInlineContextSection, offloadLargeContext } from './contextFiles.js'; @@ -14,6 +15,38 @@ You are operating in a native-tool environment, not a gadget/function-call envir - If you catch yourself composing a pseudo tool call in plain text, stop and use the real tool instead. - Trello, JIRA, and GitHub attachment URLs require backend authentication. NEVER curl, wget, or HTTP-fetch them — they return an authorization error. Work item images are pre-fetched and available either as images in your conversation context or as files under \`.cascade/context/images/\` — use whichever is present; never fetch the original URLs.`; +type PromptParamSchema = { + type: string; + required?: boolean; + default?: unknown; + description?: string; + options?: string[]; + items?: string; + aliases?: readonly string[]; + example?: unknown; +}; + +function formatExampleInvocation(key: string, schema: PromptParamSchema): string | undefined { + if (schema.example === undefined) return undefined; + + if (schema.type === 'boolean') { + return schema.example ? `--${key}` : `--no-${key}`; + } + + if (schema.type === 'object' || (schema.type === 'array' && schema.items === 'object')) { + const json = formatJsonExample(schema.example); + return json ? `--${key} ${json}` : undefined; + } + + if (schema.type === 'array') { + const examples = Array.isArray(schema.example) ? schema.example : [schema.example]; + if (examples.length === 0) return undefined; + return examples.map((value) => `--${key} ${formatShellScalar(value)}`).join(' '); + } + + return `--${key} ${formatShellScalar(schema.example)}`; +} + /** * Format a single CLI parameter for tool guidance documentation. * @@ -30,18 +63,7 @@ You are operating in a native-tool environment, not a gadget/function-call envir * - `type: 'object'` → renders as a single `''` blob. */ // biome-ignore lint/complexity/noExcessiveCognitiveComplexity: parameter-type taxonomy -function formatParam( - key: string, - schema: { - type: string; - required?: boolean; - default?: unknown; - description?: string; - items?: string; - aliases?: readonly string[]; - example?: unknown; - }, -): string { +function formatParam(key: string, schema: PromptParamSchema): string { const aliasSuffix = (schema.aliases ?? []).map((a) => `|--${a}`).join(''); const flagHead = `--${key}${aliasSuffix}`; @@ -75,15 +97,9 @@ function formatParam( // per-flag example said exactly that. The synopsis renders the toggle form; // the example reinforces it concretely. if (schema.example !== undefined) { - if (schema.type === 'boolean') { - result += schema.example ? `\n # example: --${key}` : `\n # example: --no-${key}`; - } else { - try { - result += `\n # example: --${key} '${JSON.stringify(schema.example)}'`; - } catch { - // JSON.stringify throws on cyclic refs — never in our tool definitions, - // but be defensive so a malformed example never crashes prompt building. - } + const exampleInvocation = formatExampleInvocation(key, schema); + if (exampleInvocation) { + result += `\n # example: ${exampleInvocation}`; } } @@ -112,7 +128,7 @@ export function buildToolGuidance(tools: ToolManifest[]): string { guidance += `\`\`\`bash\n${tool.cliCommand}`; for (const [key, schema] of Object.entries(tool.parameters)) { - guidance += formatParam(key, schema as { type: string; required?: boolean }); + guidance += formatParam(key, schema as PromptParamSchema); } guidance += '\n```\n\n'; diff --git a/src/cli/pm/read-work-item.ts b/src/cli/pm/read-work-item.ts index d349b10fe..9f4886243 100644 --- a/src/cli/pm/read-work-item.ts +++ b/src/cli/pm/read-work-item.ts @@ -2,6 +2,20 @@ import { readWorkItem } from '../../gadgets/pm/core/readWorkItem.js'; import { readWorkItemDef } from '../../gadgets/pm/definitions.js'; import { createCLICommand } from '../../gadgets/shared/cliCommandFactory.js'; +function normalizeWorkItemId(workItemId: string): string { + if (workItemId.length >= 2) { + const first = workItemId[0]; + const last = workItemId.at(-1); + if ((first === '"' || first === "'") && first === last) { + return workItemId.slice(1, -1); + } + } + return workItemId; +} + export default createCLICommand(readWorkItemDef, async (params) => { - return readWorkItem(params.workItemId as string, params.includeComments as boolean | undefined); + return readWorkItem( + normalizeWorkItemId(params.workItemId as string), + params.includeComments as boolean | undefined, + ); }); diff --git a/src/gadgets/README.md b/src/gadgets/README.md index d441540b4..b0848c2d0 100644 --- a/src/gadgets/README.md +++ b/src/gadgets/README.md @@ -63,11 +63,19 @@ cli: { A list of `{ params, comment, output? }` invocations. The first example that populates a given parameter becomes that parameter's **concrete example**, surfaced in three places: -- The agent's system prompt renders a one-line `# example: -- ''` under the flag (when the param is array-of-object / object). +- The agent's system prompt renders a one-line `# example: ...` under the flag. Object params and array-of-object params use a single shell-quoted JSON payload. Scalar, enum, number, and primitive-array params render as actual CLI syntax. - The `cascade-tools … --help` output lists every example as a runnable shell invocation under an `EXAMPLES` section. - JSON-parse failures include the example as the `expected` shape fragment in the structured error envelope. -Write examples that a model could literally copy/paste. Use double-quoted JSON keys; do not rely on the agent to translate pseudo-JSON. +Write examples that a model could literally copy/paste. Use double-quoted JSON keys; do not rely on the agent to translate pseudo-JSON. Shell-safe scalar IDs and names should stay bare (`--workItemId abc123`, `--owner acme`, `--prNumber 42`), not wrapped in quote characters. The shared renderer adds shell quotes only when a scalar contains spaces, shell metacharacters, or embedded quotes. + +Enum examples must be raw CLI values, not JSON strings. For example, the review action should render as: + +```bash +cascade-tools scm create-pr-review --event APPROVE +``` + +not `--event '"APPROVE"'`. Primitive arrays should render as repeated flags (`--labels bug --labels docs`), while array-of-object examples like `comments` stay JSON (`--comments '[{"path":"src/x.ts","line":1,"body":"nit"}]'`). ```ts examples: [ @@ -114,6 +122,8 @@ Envelope fields: You do not call `emitCliError` directly. The shared factory routes every failure through it automatically — your job is to make the declarative metadata (describe text, examples, aliases, file alternatives) rich enough that the auto-generated envelope is actually useful. +Core gadget functions must throw for fatal runtime/API/provider failures. Do not return sentinel prose such as `Error posting comment: ...`; `createCLICommand()` treats returned values as successful `data`, and thrown errors as runtime failure envelopes. Intentional non-fatal outcomes may still return structured data or explicit status text when they are part of the command contract, such as guarded PM move no-ops or friction reports queued for retry. + --- ## Shared CLI helper layout @@ -123,7 +133,8 @@ You do not call `emitCliError` directly. The shared factory routes every failure | Helper | Responsibility | |---|---| | `commandNames.ts` | Kebab-case conversion and `cascade-tools ` derivation shared with manifest generation | -| `examples.ts` | Example lookup, shell quoting, oclif example rendering, and JSON expected-shape hints | +| `examples.ts` | Example lookup, oclif example rendering, and JSON expected-shape hints | +| `shellValues.ts` | Shared shell-safe scalar and JSON payload formatting for CLI help and native-tool prompts | | `flags.ts` | oclif flag construction plus candidate and boolean-flag metadata collection | | `booleanArgv.ts` | Natural boolean value forms such as `--flag true`, `--flag=false`, `yes/no`, and `1/0` | | `parseErrors.ts` | oclif parse-error classification and unknown-flag suggestions | @@ -158,7 +169,7 @@ Two tuning constants live in `src/gadgets/shared/cli/parseErrors.ts`: `MAX_FLAG_ ## Reference: `createPRReviewDef` -`src/gadgets/github/definitions.ts` is the reference gadget for spec 014 — it carries `cliAliases: ['comment']` on `comments`, a `fileInputAlternatives` entry for `--comments-file`, and a well-formed `examples` block. Read it before you add a new gadget with array-of-object parameters. +`src/gadgets/github/definitions.ts` is the reference gadget for spec 014 — it carries `cliAliases: ['comment']` on `comments`, `fileInputAlternatives` entries for `--body-file` (long review summaries) and `--comments-file` (JSON inline comments), and a well-formed `examples` block. Read it before you add a new gadget with array-of-object parameters. ## Reference: `ReportFriction` diff --git a/src/gadgets/github/definitions.ts b/src/gadgets/github/definitions.ts index 53fba219a..dc81f2eeb 100644 --- a/src/gadgets/github/definitions.ts +++ b/src/gadgets/github/definitions.ts @@ -237,6 +237,11 @@ export const createPRReviewDef: ToolDefinition = { cli: { autoResolved: ownerRepoAutoResolved, fileInputAlternatives: [ + { + paramName: 'body', + fileFlag: 'body-file', + description: 'Read review body from file (use - for stdin)', + }, { paramName: 'comments', fileFlag: 'comments-file', diff --git a/src/gadgets/pm/core/deleteChecklistItem.ts b/src/gadgets/pm/core/deleteChecklistItem.ts index cb57db423..a5c3e6fa0 100644 --- a/src/gadgets/pm/core/deleteChecklistItem.ts +++ b/src/gadgets/pm/core/deleteChecklistItem.ts @@ -9,6 +9,6 @@ export async function deleteChecklistItem( return `Checklist item ${checkItemId} deleted from work item ${workItemId}`; } catch (error) { const message = error instanceof Error ? error.message : String(error); - return `Error deleting checklist item: ${message}`; + throw new Error(`Error deleting checklist item: ${message}`); } } diff --git a/src/gadgets/pm/core/listWorkItems.ts b/src/gadgets/pm/core/listWorkItems.ts index 66e073fdd..12a60fd87 100644 --- a/src/gadgets/pm/core/listWorkItems.ts +++ b/src/gadgets/pm/core/listWorkItems.ts @@ -22,6 +22,6 @@ export async function listWorkItems(containerId: string): Promise { return result; } catch (error) { const message = error instanceof Error ? error.message : String(error); - return `Error listing work items: ${message}`; + throw new Error(`Error listing work items: ${message}`); } } diff --git a/src/gadgets/pm/core/moveWorkItem.ts b/src/gadgets/pm/core/moveWorkItem.ts index a805b930e..8466fae4c 100644 --- a/src/gadgets/pm/core/moveWorkItem.ts +++ b/src/gadgets/pm/core/moveWorkItem.ts @@ -45,6 +45,6 @@ export async function moveWorkItem(params: MoveWorkItemParams): Promise return `Work item ${params.workItemId} moved to ${params.destination} successfully`; } catch (error) { const message = error instanceof Error ? error.message : String(error); - return `Error moving work item: ${message}`; + throw new Error(`Error moving work item: ${message}`); } } diff --git a/src/gadgets/pm/core/postComment.ts b/src/gadgets/pm/core/postComment.ts index f4ec81b9c..836e532d4 100644 --- a/src/gadgets/pm/core/postComment.ts +++ b/src/gadgets/pm/core/postComment.ts @@ -33,6 +33,6 @@ export async function postComment(workItemId: string, text: string): Promise 0 && SHELL_SAFE_VALUE_PATTERN.test(rendered)) { + return rendered; + } + return shellQuote(rendered); +} + +export function formatJsonExample(value: unknown): string | undefined { + try { + return shellQuote(JSON.stringify(value)); + } catch { + // JSON.stringify throws on cyclic refs. Tool definition examples should + // never be cyclic, but prompt/help rendering should not crash if one is. + return undefined; + } +} diff --git a/src/integrations/README.md b/src/integrations/README.md index 4b6aef3fd..a828507a1 100644 --- a/src/integrations/README.md +++ b/src/integrations/README.md @@ -31,12 +31,14 @@ A new PM provider is ONE manifest backed by ONE provider folder + ONE wizard fol web/src/components/projects/pm-providers// index.ts // registerProviderWizard(ProviderWizard) on module load - wizard.ts // ProviderWizardDefinition (steps, save transform, completion predicates) - adapters.tsx // step-component adapters that bridge providerHooks → existing step props - steps.tsx // React components for each wizard step (or re-export from pm-wizard--steps.tsx) + wizard.ts // ProviderWizardDefinition (steps, save transform, edit hydration) + state.ts // provider-owned WizardState slice, actions, reducer, defaults + hooks.ts // provider-owned discovery/mutation/webhook hooks + auth wrappers + webhook-step.tsx // optional provider webhook UX composition around shared steps + steps.tsx // optional custom-step components only ``` -Nothing outside those two folders needs to change when you add a provider. The registries are the only surface the rest of the codebase sees. +The registries are the only surface the rest of the codebase sees. A new provider wires itself through the backend PM barrel and the frontend PM-provider barrel; it does not edit shared wizard orchestration (`pm-wizard.tsx`, `pm-wizard-hooks.ts`, or `pm-wizard-common-steps.tsx`). The shared dashboard state file only composes provider-owned state slices and remains the temporary explicit exception documented in step 4 below. --- @@ -85,11 +87,20 @@ See [`web/src/components/projects/pm-providers/types.ts`](../../web/src/componen | `label` | Shown in the provider-select dropdown. | | `steps` | Array of `{ id, title, Component, isComplete }`. The generic wizard renders them in order. | | `buildIntegrationConfig` | Transforms wizard state into the integration config payload sent at save time. | +| `buildEditState` | Hydrates edit-mode state from saved config + configured credential keys. This is provider-owned and must never return raw credential values. | | `isSetupComplete` | `(state) => boolean`. True when the wizard can be saved. | | `useProviderHooks?` | Optional — composes the provider's React hooks (discovery, label creation, custom-field creation) inside a shell component. Return value flows into each step's `providerHooks` prop. | `ManifestProviderWizardSection` (`web/src/components/projects/pm-providers/manifest-section.tsx`) is the shell component that hosts the unconditional `useProviderHooks` call — it's only mounted when a manifest is registered for the active provider, so React's rules-of-hooks hold. +Frontend provider ownership is intentionally broad: + +- **State slice** — provider credential/config fields, action types, reducer cases, defaults, and provider-specific selectors live in `pm-providers//state.ts`. `pm-wizard-state.ts` composes those slices and is the only shared dashboard state surface that may need a new-provider edit while the shared state type still aggregates all provider fields. +- **Config serialization** — `ProviderWizardDefinition.buildIntegrationConfig` in `pm-providers//wizard.ts` is the only save-payload transform for that provider. +- **Edit hydration** — `ProviderWizardDefinition.buildEditState` restores saved provider config into wizard state and sets stored-credential flags from configured credential keys; shared `pm-wizard.tsx` only dispatches the returned partial state. +- **Auth metadata** — `auth`, `credentialPersistence`, and `formatVerificationDisplay` live in the provider wizard definition. Shared verification, credential readiness, mutation auth, and save code read this metadata instead of branching by provider. +- **Hooks and webhook UX** — discovery, label/custom-field creation, webhook list normalization, webhook create/delete controls, signing-secret fields, and provider-specific mutation wrappers live under `pm-providers//hooks.ts` and provider step adapters such as `webhook-step.tsx`. Shared `pm-wizard-hooks.ts` stays provider-agnostic. + --- ## Shared helpers (consume these; don't fork) @@ -204,13 +215,13 @@ All three real providers are now on the hardened contracts. Plan 009/4 also ship | Webhook-UX migration complete | Every PM wizard step, without exception, renders via the manifest path. Trello, JIRA, and Linear each own their webhook step via a per-provider adapter (`pm-providers//webhook-step.tsx`) — Fragment composition around the shared `WebhookUrlDisplayStep`. Trello + JIRA compose with programmatic "Create Webhook" button + active-webhooks list + delete + curl fallback (via existing `webhooks.create/list/delete({trelloOnly|jiraOnly:true})` tRPC endpoints). Linear composes with info banner + `ProjectSecretField` (`LINEAR_WEBHOOK_SECRET`) + 5-step manual setup instructions. | | Legacy deletions | `WebhookStep` + `LinearWebhookInfoPanel` + `useWebhookManagement` + `useLinearWebhookInfo` all deleted. `pm-wizard-common-steps.tsx` now only exports `SaveStep`. Legacy test file `pm-wizard-webhooks-step.test.ts` deleted — assertions moved into per-provider adapter tests. | | Parent-wizard filter | The `-webhook` id-skip filter (stopgap from plan 011/4) is gone. `renderedManifestSteps = manifestDef.steps.map(...)` — no filter. | -| New-provider guarantee | Adding a PM provider requires zero edits to `pm-wizard.tsx`, `pm-wizard-common-steps.tsx`, or `pm-wizard-hooks.ts`. Webhook UX composition lives in the provider folder. | +| New-provider guarantee | Adding a PM provider requires zero edits to `pm-wizard.tsx`, `pm-wizard-common-steps.tsx`, or `pm-wizard-hooks.ts`. New providers add one import to the frontend barrel (`web/src/components/projects/pm-providers/index.ts`) — the symmetric counterpart of the backend barrel — and `pm-wizard.tsx` picks it up automatically. The provider picker, edit hydration dispatch (`ProviderWizardDefinition.buildEditState`), config serialization (`ProviderWizardDefinition.buildIntegrationConfig`), verification-button readiness (`areCredentialsReadyFromMetadata`), mutation auth path (`buildProviderAuthArgFromMetadata`), and save credential persistence are all metadata/provider-definition driven; no shared edits required beyond the barrel import. **Shared dashboard state** (`pm-wizard-state.ts`) must still compose the new provider's state slice and action type — see step 4 of "Adding a new PM provider" below. | --- ## Adding a new PM provider (step by step) -Spec 009 AC #10: **a new PM provider PR should not need to edit shared router / worker / CLI / dashboard / configMapper / central schema files**. Everything lives in your provider folder + your wizard folder + a single import in `src/integrations/pm/index.ts`. The `tests/unit/integrations/new-provider-surface.test.ts` guard enforces this. +Spec 009 AC #10: **a new PM provider PR should not need to edit shared router / worker / CLI / dashboard / configMapper / central schema files**. The orchestration and schema work lives in your provider folder + your wizard folder + one import in `src/integrations/pm/index.ts` + one import in `web/src/components/projects/pm-providers/index.ts`. The shared wizard orchestration files (`pm-wizard.tsx`, `pm-wizard-hooks.ts`, `pm-wizard-common-steps.tsx`) are guarded shared surface and should not change for a new provider. The one shared dashboard file that still requires a new-provider edit is `pm-wizard-state.ts` (step 4 below) — it composes the provider's state slice into `WizardState`, `WizardAction`, initial state, and reducer delegation. Edit-mode config hydration belongs on the provider's `ProviderWizardDefinition.buildEditState`; save-payload serialization belongs on `ProviderWizardDefinition.buildIntegrationConfig`; provider hook auth and mutation wiring belong in provider-owned metadata/hooks. The `tests/unit/integrations/new-provider-surface.test.ts` guard enforces the shared-file invariant for orchestration and schema files; `pm-wizard-state.ts` is the deliberate exception. 1. **Backend folder** at `src/integrations/pm//`: - `client.ts` (or reuse a sibling under `src//`) — your REST / GraphQL client. Must use `withXxxCredentials()` + AsyncLocalStorage credential scoping; never hand-assemble Bearer headers (see `_shared/auth-headers.ts`). @@ -220,17 +231,28 @@ Spec 009 AC #10: **a new PM provider PR should not need to edit shared router / - `router-adapter.ts`, `triggers/*.ts`, `webhook.ts`, `platform-client.ts` — same as before. - `index.ts` — side-effect module calling `registerPMProvider(Manifest)`. -2. **Wire the manifest** via a single import in `src/integrations/pm/index.ts` (`import './/index.js';`). No other edit to any shared file is needed for registration — the `single-entrypoint` test guards this. +2. **Wire the backend manifest** via a single import in `src/integrations/pm/index.ts` (`import './/index.js';`). No other backend file needs to change — the `single-entrypoint` test guards this. + +3. **Frontend folder** at `web/src/components/projects/pm-providers//`: `wizard.ts` (`ProviderWizardDefinition` with `auth`, `credentialPersistence`, `formatVerificationDisplay`, `buildIntegrationConfig`, `buildEditState`, and `useProviderHooks` if the provider needs discovery / label creation / custom-field creation / webhook registration), `state.ts` for the provider-owned wizard state slice/actions/reducer/defaults, `hooks.ts` for provider-owned discovery/mutation/auth/webhook wrappers, `auth.ts` for reusable auth metadata when useful, and `index.ts` for side-effect registration (`registerProviderWizard(ProviderWizard)`). For shared wizard steps declared on `manifest.wizardSpec`, the generator in `pm-providers/generator.tsx` dispatches directly to the real shared step components at `pm-providers/steps/*.tsx` — there are **seven** kinds: `credentials`, `container-pick`, `status-mapping`, `label-mapping`, `webhook-url-display`, `project-scope`, `custom-field-mapping`. A provider with purely standard steps writes **zero** per-provider step components; Trello, JIRA, and Linear all use the shared components for every standard kind. Provide `providerHooks` (returned from `useProviderHooks`) to forward discovery data + mutation callbacks into the shared components; the generator spreads `ctx.providerHooks` as props. Unknown step `kind` values still warn-and-render a placeholder. **Provider-specific UI** ships either as (a) `kind: 'custom'` steps declared on the manifest and resolved to provider-folder components (Trello OAuth popup, JIRA issue-type mapping), or (b) Fragment compositions around a shared step when the base UX is standard but needs augmentation (Trello/JIRA webhook steps compose `WebhookUrlDisplayStep` + programmatic Create UX + active-webhook normalization; Linear composes `WebhookUrlDisplayStep` + `ProjectSecretField` + setup instructions — see `pm-providers/{trello,jira,linear}/webhook-step.tsx` for the reference composition pattern). Shared `pm-wizard-hooks.ts` remains limited to metadata-driven verification/save shells and provider-agnostic mutation factories. + +4. **Update shared dashboard state** in `web/src/components/projects/pm-wizard-state.ts`. This is the one shared dashboard file a new provider must edit while `WizardState` remains an aggregate type: + - Import the provider's state-slice helpers from `pm-providers//state.ts`. + - Compose the provider slice into `WizardState`, `WizardAction`, `createInitialState`, and `wizardReducer` delegation. + - Keep provider-specific credential fields, action definitions, defaults, and reducer logic inside the provider `state.ts`; reducer cases that mutate credentials should clear `verificationResult` and `verifyError`. + + Edit-mode hydration is provider-owned: implement `buildEditState(initialConfig, configuredKeys)` on the provider's `ProviderWizardDefinition`. It should set `hasStoredCredentials` from the relevant persisted credential keys and restore saved config values (container IDs, status/label mappings) without returning raw credential values. + + Save config serialization is also provider-owned: implement `buildIntegrationConfig(state)` on the provider's `ProviderWizardDefinition`. The credential-readiness check (`areCredentialsReadyFromMetadata` in `pm-wizard-hooks.ts`) and the mutation auth path (`buildProviderAuthArgFromMetadata`) are fully metadata-driven — they read `manifestDef.auth.rawCredentials` and require **no changes** here. -3. **Frontend folder** at `web/src/components/projects/pm-providers//`: `wizard.ts` (`ProviderWizardDefinition` with `useProviderHooks` if the provider needs discovery / label creation / custom-field creation / webhook registration), `index.ts`. Add one line to `pm-wizard.tsx` to register. For shared wizard steps declared on `manifest.wizardSpec`, the generator in `pm-providers/generator.tsx` dispatches directly to the real shared step components at `pm-providers/steps/*.tsx` — there are **seven** kinds: `credentials`, `container-pick`, `status-mapping`, `label-mapping`, `webhook-url-display`, `project-scope`, `custom-field-mapping`. A provider with purely standard steps writes **zero** per-provider step components; Trello, JIRA, and Linear all use the shared components for every standard kind. Provide `providerHooks` (returned from `useProviderHooks`) to forward discovery data + mutation callbacks into the shared components; the generator spreads `ctx.providerHooks` as props. Unknown step `kind` values still warn-and-render a placeholder. **Provider-specific UI** ships either as (a) `kind: 'custom'` steps declared on the manifest and resolved to provider-folder components (Trello OAuth popup, JIRA issue-type mapping), or (b) Fragment compositions around a shared step when the base UX is standard but needs augmentation (Trello/JIRA webhook steps compose `WebhookUrlDisplayStep` + programmatic Create UX; Linear composes `WebhookUrlDisplayStep` + `ProjectSecretField` + setup instructions — see `pm-providers/{trello,jira,linear}/webhook-step.tsx` for the reference composition pattern). +5. **Wire the frontend wizard** via a single import in `web/src/components/projects/pm-providers/index.ts` (`import './/index.js';`). This frontend barrel is the symmetric counterpart of the backend barrel at `src/integrations/pm/index.ts` — `pm-wizard.tsx` imports the barrel once and never needs to be edited for a new provider. -4. **Lifecycle fixture** at `tests/helpers/LifecycleFixture.ts`. Add the fixture key to `LIFECYCLE_FIXTURES` in `tests/unit/integrations/pm-conformance.test.ts`. Trivial providers can reuse `createFakePMProvider()` (see Trello/JIRA/Linear fixtures). +6. **Lifecycle fixture** at `tests/helpers/LifecycleFixture.ts`. Add the fixture key to `LIFECYCLE_FIXTURES` in `tests/unit/integrations/pm-conformance.test.ts`. Trivial providers can reuse `createFakePMProvider()` (see Trello/JIRA/Linear fixtures). -5. **Run the conformance harness**: `npx vitest run --project unit-core tests/unit/integrations/pm-conformance.test.ts`. Behavioral contracts run against your provider automatically once `configSchema` / `discoveryCapabilities` / `lifecycle` are declared. Failures name the contract. +7. **Run the conformance harness**: `npx vitest run --project unit-core tests/unit/integrations/pm-conformance.test.ts`. Behavioral contracts run against your provider automatically once `configSchema` / `discoveryCapabilities` / `lifecycle` are declared. Failures name the contract. -6. **Provider-specific unit tests** in `tests/unit/pm//` — adapter tests (vi.mock the client), config-schema round-trip, discovery shape, wizardSpec, adapter branded IDs. +8. **Provider-specific unit tests** in `tests/unit/pm//` — adapter tests (vi.mock the client), config-schema round-trip, discovery shape, wizardSpec, adapter branded IDs. -That's it. The `new-provider-surface` snapshot test proves your PR touches **no** shared infrastructure file. +The shared orchestration files (`pm-wizard.tsx`, `pm-wizard-hooks.ts`, `pm-wizard-common-steps.tsx`) require zero edits beyond the barrel import in step 5. The `new-provider-surface` snapshot test proves your PR does not modify shared router / worker / CLI / dashboard orchestration or central schema files. The one deliberate shared-dashboard exception is `pm-wizard-state.ts` for provider-specific state fields and reducer actions (step 4 above). --- diff --git a/src/trello/client.ts b/src/trello/client.ts index 94f2c9cd0..1064da0ed 100644 --- a/src/trello/client.ts +++ b/src/trello/client.ts @@ -160,10 +160,14 @@ export const trelloClient = { async updateCard(cardId: string, updates: { name?: string; desc?: string }): Promise { logger.debug('Updating card', { cardId, hasName: !!updates.name, hasDesc: !!updates.desc }); - await getClient().cards.updateCard({ - id: cardId, - name: updates.name, - desc: updates.desc, + const body: { name?: string; desc?: string } = {}; + if (updates.name !== undefined) body.name = updates.name; + if (updates.desc !== undefined) body.desc = updates.desc; + + await trelloFetch(`/cards/${cardId}`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body, }); }, diff --git a/tests/unit/backends/shared-nativeToolPrompts.test.ts b/tests/unit/backends/shared-nativeToolPrompts.test.ts index 83db9991e..e788c7ad5 100644 --- a/tests/unit/backends/shared-nativeToolPrompts.test.ts +++ b/tests/unit/backends/shared-nativeToolPrompts.test.ts @@ -25,6 +25,9 @@ import { buildTaskPrompt, buildToolGuidance, } from '../../../src/backends/shared/nativeToolPrompts.js'; +import { createPRReviewDef } from '../../../src/gadgets/github/definitions.js'; +import { readWorkItemDef } from '../../../src/gadgets/pm/definitions.js'; +import { generateToolManifest } from '../../../src/gadgets/shared/manifestGenerator.js'; // ───────── helper ───────── function makeManifest(overrides: Partial = {}): ToolManifest { @@ -114,6 +117,79 @@ describe('buildToolGuidance', () => { }); }); + describe('formatParam — enum/scalar examples', () => { + it('renders enum examples as raw CLI values instead of JSON string literals', () => { + const result = buildToolGuidance([ + makeManifest({ + parameters: { + event: { + type: 'string', + options: ['APPROVE', 'REQUEST_CHANGES', 'COMMENT'], + required: true, + example: 'APPROVE', + }, + }, + }), + ]); + + expect(result).toContain('# example: --event APPROVE'); + expect(result).not.toContain(`--event '"APPROVE"'`); + expect(result).not.toContain(`--event '"REQUEST_CHANGES"'`); + }); + + it('renders number examples as raw CLI values', () => { + const result = buildToolGuidance([ + makeManifest({ + parameters: { + prNumber: { type: 'number', required: true, example: 42 }, + }, + }), + ]); + + expect(result).toContain('# example: --prNumber 42'); + expect(result).not.toContain(`--prNumber '42'`); + }); + + it('shell-quotes scalar string examples only when needed', () => { + const result = buildToolGuidance([ + makeManifest({ + parameters: { + body: { + type: 'string', + required: true, + example: 'LGTM after CI passes', + }, + }, + }), + ]); + + expect(result).toContain("# example: --body 'LGTM after CI passes'"); + }); + + it('renders CreatePRReview event guidance without JSON string quotes', () => { + const result = buildToolGuidance([generateToolManifest(createPRReviewDef)]); + + expect(result).toContain('# example: --event APPROVE'); + expect(result).not.toContain(`--event '"APPROVE"'`); + }); + + it('renders ReadWorkItem work item IDs as bare shell-safe values', () => { + const result = buildToolGuidance([generateToolManifest(readWorkItemDef)]); + + expect(result).toContain('# example: --workItemId abc123'); + expect(result).not.toContain(`--workItemId '"abc123"'`); + expect(result).not.toContain(`--workItemId 'abc123'`); + expect(result).not.toContain(`--workItemId "abc123"`); + }); + + it('renders CreatePRReview body-file guidance from definition metadata', () => { + const result = buildToolGuidance([generateToolManifest(createPRReviewDef)]); + + expect(result).toContain('[--body-file ]'); + expect(result).toContain('Read review body from file (use - for stdin)'); + }); + }); + describe('formatParam — optional string param', () => { it('formats optional string param with brackets', () => { const result = buildToolGuidance([ @@ -156,6 +232,24 @@ describe('buildToolGuidance', () => { expect(result).toContain('[--labels (repeatable)]'); expect(result).not.toContain('--label '); // old bug }); + + it('renders primitive-array examples as repeated flags instead of a JSON blob', () => { + const result = buildToolGuidance([ + makeManifest({ + parameters: { + labels: { + type: 'array', + items: 'string', + required: false, + example: ['bug', 'docs'], + }, + }, + }), + ]); + + expect(result).toContain('# example: --labels bug --labels docs'); + expect(result).not.toContain(`--labels '["bug","docs"]'`); + }); }); describe('formatParam — array of object (items:"object") — JSON blob (spec 014)', () => { @@ -227,6 +321,18 @@ describe('buildToolGuidance', () => { expect(result).toContain("--config ''"); expect(result).not.toContain(''); // shouldn't leak the bare type }); + + it('renders object examples as a single JSON payload', () => { + const result = buildToolGuidance([ + makeManifest({ + parameters: { + config: { type: 'object', required: true, example: { mode: 'strict' } }, + }, + }), + ]); + + expect(result).toContain(`# example: --config '${JSON.stringify({ mode: 'strict' })}'`); + }); }); describe('formatParam — boolean param', () => { diff --git a/tests/unit/cli/cascade-tools-help.test.ts b/tests/unit/cli/cascade-tools-help.test.ts index 397fd0efc..0e460b2d5 100644 --- a/tests/unit/cli/cascade-tools-help.test.ts +++ b/tests/unit/cli/cascade-tools-help.test.ts @@ -72,5 +72,8 @@ describe('cascade-tools --help — topic summaries', () => { expect(stdout).toContain('Read a work item'); expect(stdout).toContain('--workItemId'); expect(stdout).toContain('--[no-]includeComments'); + expect(stdout).toContain('cascade-tools pm read-work-item --workItemId abc123'); + expect(stdout).not.toContain(`--workItemId '"abc123"'`); + expect(stdout).not.toContain(`--workItemId 'abc123'`); }); }); diff --git a/tests/unit/cli/cascade-tools-stdout-cleanliness.test.ts b/tests/unit/cli/cascade-tools-stdout-cleanliness.test.ts index c232c1e27..4a9adc605 100644 --- a/tests/unit/cli/cascade-tools-stdout-cleanliness.test.ts +++ b/tests/unit/cli/cascade-tools-stdout-cleanliness.test.ts @@ -79,6 +79,9 @@ describe('cascade-tools — stdout is reserved for the JSON envelope', () => { ); expect(stdout).toMatch(ENVELOPE_START); + const envelope = JSON.parse(stdout.trim()); + expect(envelope.success).toBe(false); + expect(envelope.error.type).toBe('runtime'); expect(stdout).not.toContain(ESC); expect(stdout).not.toMatch(LOG_LEVEL_PREFIX); expect(stdout).not.toContain('[cascade]'); diff --git a/tests/unit/cli/dashboard/config.test.ts b/tests/unit/cli/dashboard/config.test.ts index ea6d841bc..754b15b10 100644 --- a/tests/unit/cli/dashboard/config.test.ts +++ b/tests/unit/cli/dashboard/config.test.ts @@ -21,11 +21,21 @@ import { describe('config', () => { const originalEnv = process.env; + const DASHBOARD_CONFIG_ENV_KEYS = [ + 'CASCADE_SERVER_URL', + 'CASCADE_SESSION_TOKEN', + 'CASCADE_ORG_ID', + ] as const; + + function clearDashboardConfigEnv() { + for (const key of DASHBOARD_CONFIG_ENV_KEYS) { + delete process.env[key]; + } + } beforeEach(() => { process.env = { ...originalEnv }; - delete process.env.CASCADE_SERVER_URL; - delete process.env.CASCADE_SESSION_TOKEN; + clearDashboardConfigEnv(); }); afterEach(() => { @@ -48,6 +58,22 @@ describe('config', () => { expect(existsSync).not.toHaveBeenCalled(); }); + it('returns env orgId when env config is complete', () => { + process.env.CASCADE_SERVER_URL = 'http://env-server:3000'; + process.env.CASCADE_SESSION_TOKEN = 'env-token'; + process.env.CASCADE_ORG_ID = 'env-org'; + + const config = loadConfig(); + + expect(config).toEqual({ + serverUrl: 'http://env-server:3000', + sessionToken: 'env-token', + cookieName: SESSION_COOKIE_NAME, + orgId: 'env-org', + }); + expect(existsSync).not.toHaveBeenCalled(); + }); + it('returns null when config file does not exist', () => { vi.mocked(existsSync).mockReturnValue(false); @@ -71,6 +97,26 @@ describe('config', () => { expect(readFileSync).toHaveBeenCalledWith(expect.stringContaining('cli.json'), 'utf-8'); }); + it('reads stored orgId from file when no env override is set', () => { + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + serverUrl: 'http://localhost:3000', + sessionToken: 'file-token', + orgId: 'file-org', + }), + ); + + const config = loadConfig(); + + expect(config).toEqual({ + serverUrl: 'http://localhost:3000', + sessionToken: 'file-token', + cookieName: SESSION_COOKIE_NAME, + orgId: 'file-org', + }); + }); + it('returns null when file has incomplete config', () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFileSync).mockReturnValue(JSON.stringify({ serverUrl: 'http://x' })); @@ -119,6 +165,27 @@ describe('config', () => { }); }); + it('env orgId overrides stored orgId while preserving file server and token', () => { + process.env.CASCADE_ORG_ID = 'env-org'; + vi.mocked(existsSync).mockReturnValue(true); + vi.mocked(readFileSync).mockReturnValue( + JSON.stringify({ + serverUrl: 'http://file:3000', + sessionToken: 'file-token', + orgId: 'file-org', + }), + ); + + const config = loadConfig(); + + expect(config).toEqual({ + serverUrl: 'http://file:3000', + sessionToken: 'file-token', + cookieName: SESSION_COOKIE_NAME, + orgId: 'env-org', + }); + }); + it('loads custom cookie name from file', () => { vi.mocked(existsSync).mockReturnValue(true); vi.mocked(readFileSync).mockReturnValue( diff --git a/tests/unit/cli/file-input-flags.test.ts b/tests/unit/cli/file-input-flags.test.ts index 327a78558..2df3b30d8 100644 --- a/tests/unit/cli/file-input-flags.test.ts +++ b/tests/unit/cli/file-input-flags.test.ts @@ -39,6 +39,11 @@ vi.mock('../../../src/gadgets/pm/core/postComment.js', () => ({ vi.mock('../../../src/gadgets/github/core/createPR.js', () => ({ createPR: vi.fn().mockResolvedValue({ url: 'https://github.com/o/r/pull/1' }), })); +vi.mock('../../../src/gadgets/github/core/createPRReview.js', () => ({ + createPRReview: vi + .fn() + .mockResolvedValue({ reviewUrl: 'https://github.com/o/r/pull/1#review', event: 'COMMENT' }), +})); vi.mock('../../../src/gadgets/github/core/postPRComment.js', () => ({ postPRComment: vi.fn().mockResolvedValue({ id: 123 }), })); @@ -48,8 +53,10 @@ import PostComment from '../../../src/cli/pm/post-comment.js'; import ReportFriction from '../../../src/cli/pm/report-friction.js'; import UpdateWorkItem from '../../../src/cli/pm/update-work-item.js'; import CreatePR from '../../../src/cli/scm/create-pr.js'; +import CreatePRReview from '../../../src/cli/scm/create-pr-review.js'; import PostPRComment from '../../../src/cli/scm/post-pr-comment.js'; import { createPR } from '../../../src/gadgets/github/core/createPR.js'; +import { createPRReview } from '../../../src/gadgets/github/core/createPRReview.js'; import { postPRComment } from '../../../src/gadgets/github/core/postPRComment.js'; import { createWorkItem } from '../../../src/gadgets/pm/core/createWorkItem.js'; import { postComment } from '../../../src/gadgets/pm/core/postComment.js'; @@ -352,6 +359,82 @@ describe('CreatePR --body-file', () => { }); }); +describe('CreatePRReview --body-file', () => { + const originalEnv = process.env; + + beforeEach(() => { + process.env = { + ...originalEnv, + CASCADE_REPO_OWNER: 'owner', + CASCADE_REPO_NAME: 'repo', + }; + }); + + afterEach(() => { + process.env = originalEnv; + }); + + it('reads review body from file', async () => { + const filePath = writeTempFile('review.md', 'Review body from file'); + const cmd = new CreatePRReview( + ['--prNumber', '42', '--event', 'COMMENT', '--body-file', filePath], + mockConfig as never, + ); + await cmd.run(); + + expect(createPRReview).toHaveBeenCalledWith( + expect.objectContaining({ + owner: 'owner', + repo: 'repo', + prNumber: 42, + event: 'COMMENT', + body: 'Review body from file', + }), + ); + }); + + it('prefers --body-file over --body', async () => { + const filePath = writeTempFile('review.md', 'from file'); + const cmd = new CreatePRReview( + ['--prNumber', '42', '--event', 'APPROVE', '--body', 'from flag', '--body-file', filePath], + mockConfig as never, + ); + await cmd.run(); + + expect(createPRReview).toHaveBeenCalledWith( + expect.objectContaining({ + body: 'from file', + }), + ); + }); + + it('keeps inline comments JSON parsing when combined with --body-file', async () => { + const filePath = writeTempFile('review.md', 'Needs a small change'); + const comments = [{ path: 'src/index.ts', line: 12, body: 'Please handle null here.' }]; + const cmd = new CreatePRReview( + [ + '--prNumber', + '42', + '--event', + 'REQUEST_CHANGES', + '--body-file', + filePath, + '--comments', + JSON.stringify(comments), + ], + mockConfig as never, + ); + await cmd.run(); + + expect(createPRReview).toHaveBeenCalledWith( + expect.objectContaining({ + body: 'Needs a small change', + comments, + }), + ); + }); +}); + describe('PostPRComment --body-file', () => { const originalEnv = process.env; diff --git a/tests/unit/cli/pm/pm-commands.test.ts b/tests/unit/cli/pm/pm-commands.test.ts index 5e4f9a7cf..d9af24b00 100644 --- a/tests/unit/cli/pm/pm-commands.test.ts +++ b/tests/unit/cli/pm/pm-commands.test.ts @@ -87,6 +87,14 @@ function makeMockConfig() { return { runHook: vi.fn().mockResolvedValue({ successes: [], failures: [] }) }; } +async function runExpectingExit(cmd: { run: () => Promise }): Promise { + try { + await cmd.run(); + } catch { + // emitCliError exits with code 1; oclif throws in the test environment. + } +} + afterEach(() => { vi.restoreAllMocks(); }); @@ -103,6 +111,23 @@ describe('ReadWorkItem command', () => { expect(readWorkItem).toHaveBeenCalledWith('card-123', true); }); + it('normalizes one accidental outer quote layer around workItemId', async () => { + const doubleQuoted = new ReadWorkItem( + ['--workItemId', '"card-123"'], + makeMockConfig() as never, + ); + await doubleQuoted.run(); + + const singleQuoted = new ReadWorkItem( + ['--workItemId', "'card-456'"], + makeMockConfig() as never, + ); + await singleQuoted.run(); + + expect(readWorkItem).toHaveBeenNthCalledWith(1, 'card-123', true); + expect(readWorkItem).toHaveBeenNthCalledWith(2, 'card-456', true); + }); + it('passes includeComments=true when --includeComments is set', async () => { const cmd = new ReadWorkItem( ['--workItemId', 'card-123', '--includeComments'], @@ -134,6 +159,25 @@ describe('ReadWorkItem command', () => { expect(output.success).toBe(true); expect(output.data).toEqual({ id: 'card-123', title: 'Test Card' }); }); + + it('outputs a runtime failure envelope when readWorkItem rejects', async () => { + vi.mocked(readWorkItem).mockRejectedValue(new Error('Request failed with status code 400')); + const cmd = new ReadWorkItem(['--workItemId', 'card-123'], makeMockConfig() as never); + const logSpy = vi.spyOn(cmd, 'log'); + const exitSpy = vi.spyOn(cmd, 'exit'); + + await runExpectingExit(cmd); + + expect(exitSpy).toHaveBeenCalledWith(1); + const output = JSON.parse(logSpy.mock.calls[0][0] as string); + expect(output).toEqual({ + success: false, + error: { + type: 'runtime', + message: 'Request failed with status code 400', + }, + }); + }); }); // --------------------------------------------------------------------------- @@ -399,4 +443,26 @@ describe('PostComment command (basic params)', () => { expect(output.success).toBe(true); expect(output.data).toEqual({ id: 'comment-new' }); }); + + it('outputs a runtime failure envelope when postComment rejects', async () => { + vi.mocked(postComment).mockRejectedValue(new Error('Request failed with status code 400')); + const cmd = new PostComment( + ['--workItemId', 'card-1', '--text', 'Hello world'], + makeMockConfig() as never, + ); + const logSpy = vi.spyOn(cmd, 'log'); + const exitSpy = vi.spyOn(cmd, 'exit'); + + await runExpectingExit(cmd); + + expect(exitSpy).toHaveBeenCalledWith(1); + const output = JSON.parse(logSpy.mock.calls[0][0] as string); + expect(output).toEqual({ + success: false, + error: { + type: 'runtime', + message: 'Request failed with status code 400', + }, + }); + }); }); diff --git a/tests/unit/gadgets/github/definitions.test.ts b/tests/unit/gadgets/github/definitions.test.ts index e9ba5f002..2d1cce598 100644 --- a/tests/unit/gadgets/github/definitions.test.ts +++ b/tests/unit/gadgets/github/definitions.test.ts @@ -281,9 +281,16 @@ describe('createPRReviewDef — spec 014 opt-in', () => { expect(comments.cliAliases).toEqual(['comment']); }); - it('cli.fileInputAlternatives includes a comments-file entry with parseAs:"json"', () => { + it('cli.fileInputAlternatives includes body-file and comments-file entries', () => { const alts = createPRReviewDef.cli?.fileInputAlternatives ?? []; + const bodyFile = alts.find((a) => a.paramName === 'body'); const commentsFile = alts.find((a) => a.paramName === 'comments'); + + expect(bodyFile).toBeDefined(); + expect(bodyFile?.fileFlag).toBe('body-file'); + expect(bodyFile?.parseAs).toBeUndefined(); + expect(bodyFile?.description).toBeTruthy(); + expect(commentsFile).toBeDefined(); expect(commentsFile?.fileFlag).toBe('comments-file'); expect(commentsFile?.parseAs).toBe('json'); diff --git a/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts b/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts index d31686162..5b93d8d4a 100644 --- a/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts +++ b/tests/unit/gadgets/pm/core/deleteChecklistItem.test.ts @@ -20,19 +20,19 @@ describe('deleteChecklistItem', () => { expect(result).toBe('Checklist item checkItem1 deleted from work item item1'); }); - it('returns error message on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.deleteChecklistItem.mockRejectedValue(new Error('API error')); - const result = await deleteChecklistItem('item1', 'checkItem1'); - - expect(result).toBe('Error deleting checklist item: API error'); + await expect(deleteChecklistItem('item1', 'checkItem1')).rejects.toThrow( + 'Error deleting checklist item: API error', + ); }); it('handles non-Error thrown value', async () => { mockProvider.deleteChecklistItem.mockRejectedValue('string error'); - const result = await deleteChecklistItem('item1', 'ci1'); - - expect(result).toBe('Error deleting checklist item: string error'); + await expect(deleteChecklistItem('item1', 'ci1')).rejects.toThrow( + 'Error deleting checklist item: string error', + ); }); }); diff --git a/tests/unit/gadgets/pm/core/listWorkItems.test.ts b/tests/unit/gadgets/pm/core/listWorkItems.test.ts index 144e9747e..b11f77f65 100644 --- a/tests/unit/gadgets/pm/core/listWorkItems.test.ts +++ b/tests/unit/gadgets/pm/core/listWorkItems.test.ts @@ -100,11 +100,9 @@ describe('listWorkItems', () => { expect(result).not.toContain('...'); }); - it('returns error message on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.listWorkItems.mockRejectedValue(new Error('API error')); - const result = await listWorkItems('list1'); - - expect(result).toBe('Error listing work items: API error'); + await expect(listWorkItems('list1')).rejects.toThrow('Error listing work items: API error'); }); }); diff --git a/tests/unit/gadgets/pm/core/moveWorkItem.test.ts b/tests/unit/gadgets/pm/core/moveWorkItem.test.ts index 395bb3a76..480c95b01 100644 --- a/tests/unit/gadgets/pm/core/moveWorkItem.test.ts +++ b/tests/unit/gadgets/pm/core/moveWorkItem.test.ts @@ -23,26 +23,26 @@ describe('moveWorkItem', () => { expect(result).toBe('Work item card1 moved to list2 successfully'); }); - it('returns error string on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.moveWorkItem.mockRejectedValue(new Error('API error')); - const result = await moveWorkItem({ - workItemId: 'card1', - destination: 'list2', - }); - - expect(result).toBe('Error moving work item: API error'); + await expect( + moveWorkItem({ + workItemId: 'card1', + destination: 'list2', + }), + ).rejects.toThrow('Error moving work item: API error'); }); it('handles non-Error throws', async () => { mockProvider.moveWorkItem.mockRejectedValue('network timeout'); - const result = await moveWorkItem({ - workItemId: 'card1', - destination: 'list2', - }); - - expect(result).toBe('Error moving work item: network timeout'); + await expect( + moveWorkItem({ + workItemId: 'card1', + destination: 'list2', + }), + ).rejects.toThrow('Error moving work item: network timeout'); }); // ── expectedSourceState guard ──────────────────────────────────────────── @@ -144,17 +144,18 @@ describe('moveWorkItem', () => { expect(mockProvider.moveWorkItem).toHaveBeenCalledWith('card1', 'list2'); }); - it('returns a structured error if getWorkItem throws', async () => { + it('throws a structured error if getWorkItem throws', async () => { mockProvider.getWorkItem.mockRejectedValue(new Error('API down')); - const result = await moveWorkItem({ - workItemId: 'MNG-538', - destination: 'todo-state-id', - expectedSourceState: 'Backlog', - }); + await expect( + moveWorkItem({ + workItemId: 'MNG-538', + destination: 'todo-state-id', + expectedSourceState: 'Backlog', + }), + ).rejects.toThrow('Error moving work item: API down'); expect(mockProvider.moveWorkItem).not.toHaveBeenCalled(); - expect(result).toContain('Error'); }); }); }); diff --git a/tests/unit/gadgets/pm/core/postComment.test.ts b/tests/unit/gadgets/pm/core/postComment.test.ts index 64f7eb00d..8618d13a2 100644 --- a/tests/unit/gadgets/pm/core/postComment.test.ts +++ b/tests/unit/gadgets/pm/core/postComment.test.ts @@ -48,12 +48,12 @@ describe('postComment', () => { expect(result).toBe('Comment posted successfully'); }); - it('returns error message on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.addComment.mockRejectedValue(new Error('Network error')); - const result = await postComment('item1', 'text'); - - expect(result).toBe('Error posting comment: Network error'); + await expect(postComment('item1', 'text')).rejects.toThrow( + 'Error posting comment: Network error', + ); }); it('passes multi-line text correctly', async () => { @@ -68,9 +68,9 @@ describe('postComment', () => { it('handles non-Error thrown value', async () => { mockProvider.addComment.mockRejectedValue('string error'); - const result = await postComment('item1', 'text'); - - expect(result).toBe('Error posting comment: string error'); + await expect(postComment('item1', 'text')).rejects.toThrow( + 'Error posting comment: string error', + ); }); describe('progress comment replacement', () => { diff --git a/tests/unit/gadgets/pm/core/readWorkItem.test.ts b/tests/unit/gadgets/pm/core/readWorkItem.test.ts index 4767c99fd..d04e3c0fb 100644 --- a/tests/unit/gadgets/pm/core/readWorkItem.test.ts +++ b/tests/unit/gadgets/pm/core/readWorkItem.test.ts @@ -159,12 +159,10 @@ describe('readWorkItem', () => { expect(mockProvider.getWorkItemComments).not.toHaveBeenCalled(); }); - it('returns error message on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.getWorkItem.mockRejectedValue(new Error('Network error')); - const result = await readWorkItem('item1'); - - expect(result).toContain('Error reading work item: Network error'); + await expect(readWorkItem('item1')).rejects.toThrow('Error reading work item: Network error'); }); it('handles label without color', async () => { diff --git a/tests/unit/gadgets/pm/core/updateChecklistItem.test.ts b/tests/unit/gadgets/pm/core/updateChecklistItem.test.ts index 99619a59e..da22e30ef 100644 --- a/tests/unit/gadgets/pm/core/updateChecklistItem.test.ts +++ b/tests/unit/gadgets/pm/core/updateChecklistItem.test.ts @@ -29,19 +29,19 @@ describe('updateChecklistItem', () => { expect(result).toBe('Checklist item checkItem1 marked incomplete on work item item1'); }); - it('returns error message on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.updateChecklistItem.mockRejectedValue(new Error('API error')); - const result = await updateChecklistItem('item1', 'checkItem1', true); - - expect(result).toBe('Error updating checklist item: API error'); + await expect(updateChecklistItem('item1', 'checkItem1', true)).rejects.toThrow( + 'Error updating checklist item: API error', + ); }); it('handles non-Error thrown value', async () => { mockProvider.updateChecklistItem.mockRejectedValue('string error'); - const result = await updateChecklistItem('item1', 'ci1', false); - - expect(result).toBe('Error updating checklist item: string error'); + await expect(updateChecklistItem('item1', 'ci1', false)).rejects.toThrow( + 'Error updating checklist item: string error', + ); }); }); diff --git a/tests/unit/gadgets/pm/core/updateWorkItem.test.ts b/tests/unit/gadgets/pm/core/updateWorkItem.test.ts index 6a28eb65e..12f889c4d 100644 --- a/tests/unit/gadgets/pm/core/updateWorkItem.test.ts +++ b/tests/unit/gadgets/pm/core/updateWorkItem.test.ts @@ -75,12 +75,12 @@ describe('updateWorkItem', () => { expect(result).toBe('Work item updated: title, description, 1 label(s)'); }); - it('returns error message on failure', async () => { + it('throws an error message on failure', async () => { mockProvider.updateWorkItem.mockRejectedValue(new Error('API error')); - const result = await updateWorkItem({ workItemId: 'item1', title: 'T' }); - - expect(result).toBe('Error updating work item: API error'); + await expect(updateWorkItem({ workItemId: 'item1', title: 'T' })).rejects.toThrow( + 'Error updating work item: API error', + ); }); it('does not call updateWorkItem when only labels provided', async () => { diff --git a/tests/unit/gadgets/shared/cli/examples.test.ts b/tests/unit/gadgets/shared/cli/examples.test.ts index ad2442b51..fd9694d65 100644 --- a/tests/unit/gadgets/shared/cli/examples.test.ts +++ b/tests/unit/gadgets/shared/cli/examples.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; - +import { readWorkItemDef } from '../../../../../src/gadgets/pm/definitions.js'; import { buildOclifExamples, expectedShapeFor, @@ -47,11 +47,20 @@ describe('CLI examples', () => { const [line] = buildOclifExamples(reviewDef, 'cascade-tools scm create-pr-review'); expect(line).toContain("cascade-tools scm create-pr-review --body 'Don'\\''t merge yet'"); expect(line).toContain('--no-draft'); - expect(line).toContain("--labels 'needs-work' --labels 'api'"); + expect(line).toContain('--labels needs-work --labels api'); expect(line).toContain('--comments \'[{"path":"src/a.ts","line":3,"body":"nit"}]\''); expect(line).not.toContain('hidden'); }); + it('renders ReadWorkItem examples with shell-safe IDs as bare values', () => { + const [line] = buildOclifExamples(readWorkItemDef, 'cascade-tools pm read-work-item'); + + expect(line).toBe('cascade-tools pm read-work-item --workItemId abc123 --includeComments'); + expect(line).not.toContain(`--workItemId '"abc123"'`); + expect(line).not.toContain(`--workItemId 'abc123'`); + expect(line).not.toContain(`--workItemId "abc123"`); + }); + it('uses examples as JSON expected-shape hints with describe fallback', () => { const paramDef = reviewDef.parameters.comments; expect(expectedShapeFor(paramDef, [{ path: 'x.ts' }])).toBe('[{"path":"x.ts"}]'); diff --git a/tests/unit/gadgets/shared/promptExamplesGrammar.test.ts b/tests/unit/gadgets/shared/promptExamplesGrammar.test.ts index 82abb156d..84eb22d1a 100644 --- a/tests/unit/gadgets/shared/promptExamplesGrammar.test.ts +++ b/tests/unit/gadgets/shared/promptExamplesGrammar.test.ts @@ -21,7 +21,9 @@ import { describe, expect, it } from 'vitest'; import { buildToolGuidance } from '../../../../src/backends/shared/nativeToolPrompts.js'; import * as githubDefs from '../../../../src/gadgets/github/definitions.js'; +import { createPRReviewDef } from '../../../../src/gadgets/github/definitions.js'; import * as pmDefs from '../../../../src/gadgets/pm/definitions.js'; +import { readWorkItemDef } from '../../../../src/gadgets/pm/definitions.js'; import * as sentryDefs from '../../../../src/gadgets/sentry/definitions.js'; import * as sessionDefs from '../../../../src/gadgets/session/definitions.js'; import { generateToolManifest } from '../../../../src/gadgets/shared/manifestGenerator.js'; @@ -83,4 +85,65 @@ describe('prompt-rendered examples — CLI grammar correctness', () => { } }); } + + it('CreatePRReview renders enum examples as raw CLI values', () => { + const manifest = generateToolManifest(createPRReviewDef); + const rendered = buildToolGuidance([manifest]); + + expect(rendered).toContain('# example: --event APPROVE'); + expect(rendered).not.toContain(`--event '"APPROVE"'`); + expect(rendered).not.toContain(`--event '"REQUEST_CHANGES"'`); + expect(rendered).not.toContain(`--event '"COMMENT"'`); + }); + + it('ReadWorkItem renders shell-safe PM IDs without literal quotes', () => { + const manifest = generateToolManifest(readWorkItemDef); + const rendered = buildToolGuidance([manifest]); + + expect(rendered).toContain('# example: --workItemId abc123'); + expect(rendered).not.toContain(`--workItemId '"abc123"'`); + expect(rendered).not.toContain(`--workItemId 'abc123'`); + expect(rendered).not.toContain(`--workItemId "abc123"`); + }); + + it('CreatePRReview keeps array-of-object comments as JSON payload examples', () => { + const manifest = generateToolManifest(createPRReviewDef); + const rendered = buildToolGuidance([manifest]); + + expect(rendered).toContain( + `# example: --comments '${JSON.stringify([ + { + path: 'src/utils.ts', + line: 15, + body: 'This could cause a null pointer exception. Please add a null check.', + }, + ])}'`, + ); + expect(rendered).not.toContain('--comments (repeatable)'); + expect(rendered).not.toContain('--comments path'); + }); + + for (const def of allDefs) { + const enumFlagNames = Object.entries(def.parameters) + .filter(([, p]) => p.type === 'enum' && !p.gadgetOnly) + .map(([k]) => k); + + if (enumFlagNames.length === 0) continue; + + it(`${def.name}: enum flag examples use raw CLI grammar`, () => { + const manifest = generateToolManifest(def); + const rendered = buildToolGuidance([manifest]); + + for (const flagName of enumFlagNames) { + const options = + def.parameters[flagName]?.type === 'enum' ? def.parameters[flagName].options : []; + for (const option of options) { + expect( + rendered, + `${def.name}.${flagName} must not render --${flagName} '"${option}"'`, + ).not.toContain(`--${flagName} '"${option}"'`); + } + } + }); + } }); diff --git a/tests/unit/integrations/new-provider-surface.test.ts b/tests/unit/integrations/new-provider-surface.test.ts index 3a83dbe20..665c5f658 100644 --- a/tests/unit/integrations/new-provider-surface.test.ts +++ b/tests/unit/integrations/new-provider-surface.test.ts @@ -13,8 +13,19 @@ * guard, not a hard ban — if a contributor genuinely needs to extend * shared infrastructure (e.g., adding a new StandardStepKind), they * update the expected list below and explain why in the commit message. + * + * The three shared PM wizard orchestration files (pm-wizard.tsx, + * pm-wizard-hooks.ts, pm-wizard-common-steps.tsx) receive an additional + * SHA-256 content-hash guard (see GUARDED_WIZARD_FILE_HASHES below). + * Unlike the existence check, the hash check fails when the file is + * *modified*, not just when it is deleted — matching the documented + * invariant that adding a provider should never require editing them. + * Update a pinned hash only when the file genuinely needs to change for + * non-provider-specific reasons, and include the justification in the + * commit message. */ +import { createHash } from 'node:crypto'; import { readFileSync, statSync } from 'node:fs'; import { resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; @@ -44,6 +55,21 @@ const SHARED_SURFACE_FILES = [ 'src/api/routers/pm-discovery.ts', 'web/src/components/projects/pm-providers/generator.tsx', + // Shared PM wizard orchestration — provider picker, edit hydration + // dispatch, verification, save, and common save step are metadata-driven + // or provider-definition driven. New-provider frontend work belongs in + // web/src/components/projects/pm-providers//, with + // pm-wizard-state.ts as the only deliberate shared dashboard exception + // while it composes provider state slices into the aggregate WizardState. + 'web/src/components/projects/pm-wizard.tsx', + 'web/src/components/projects/pm-wizard-hooks.ts', + 'web/src/components/projects/pm-wizard-common-steps.tsx', + + // Frontend PM provider barrel — new providers add one import here, + // just like the backend barrel at src/integrations/pm/index.ts. + // pm-wizard.tsx imports this barrel and never needs to change. + 'web/src/components/projects/pm-providers/index.ts', + // Shared wizard step components (plan 010/3 + plan 011/1) — real // components for every StandardStepKind. A new provider with purely // standard steps never touches these files; it declares @@ -64,6 +90,27 @@ const SHARED_SURFACE_FILES = [ 'src/db/repositories/configMapper.ts', ] as const; +/** + * Pinned SHA-256 content hashes for the shared PM wizard orchestration + * files that a new provider PR must NEVER modify. The existence check in + * SHARED_SURFACE_FILES above only catches deletion; these hashes catch any + * modification — matching the README/architecture claim that these files + * are "guarded shared surface". + * + * To update a hash (legitimate infrastructure change, not a new-provider + * addition): run `node -e "const c=require('crypto'),f=require('fs'); + * console.log(c.createHash('sha256').update(f.readFileSync('','utf8')).digest('hex'))"`, + * paste the new value here, and include the justification in the commit message. + */ +const GUARDED_WIZARD_FILE_HASHES: Record = { + 'web/src/components/projects/pm-wizard.tsx': + '402cc6829689f34dfec940034f8ba014fe14425671018d0455ad67145e6a0fb9', + 'web/src/components/projects/pm-wizard-hooks.ts': + '7aa07ab092695afdfd2ecc78345c097b569adce46e3eb928e8ee0bfa5d4131dd', + 'web/src/components/projects/pm-wizard-common-steps.tsx': + '0d9ca8bb56036687aed695b502be75ebf2a753195decb2e6b58f440c2abaa7c9', +}; + describe('new-provider-surface (plan 009/5 task 4, spec 009 AC #10)', () => { it.each(SHARED_SURFACE_FILES)('shared surface file exists: %s', (relativePath) => { const full = resolve(PROJECT_ROOT, relativePath); @@ -81,6 +128,43 @@ describe('new-provider-surface (plan 009/5 task 4, spec 009 AC #10)', () => { } }); + /** + * Content-hash guard for the shared PM wizard orchestration files. + * Unlike the existence check above, this assertion fails the moment + * any of the three guarded files is modified — making the "guarded + * shared surface" claim in the README and architecture docs accurate. + * + * A legitimate change to one of these files (e.g., fixing a shared + * wizard bug, adding a StandardStepKind) must update the corresponding + * hash in GUARDED_WIZARD_FILE_HASHES and include a justification in + * the commit message explaining why this is not a new-provider edit. + */ + it.each( + Object.entries(GUARDED_WIZARD_FILE_HASHES), + )('shared wizard orchestration file is unmodified: %s', (relativePath, expectedHash) => { + const full = resolve(PROJECT_ROOT, relativePath); + const content = readFileSync(full, 'utf8'); + const actualHash = createHash('sha256').update(content).digest('hex'); + expect( + actualHash, + [ + `Shared wizard orchestration file ${relativePath} has been modified.`, + `Expected SHA-256: ${expectedHash}`, + `Actual SHA-256: ${actualHash}`, + ``, + `Spec 009 AC #10: adding a new PM provider must NOT require editing`, + `pm-wizard.tsx, pm-wizard-hooks.ts, or pm-wizard-common-steps.tsx.`, + `All new-provider frontend work belongs in:`, + ` web/src/components/projects/pm-providers//`, + ` (wizard.ts, state.ts, hooks.ts, auth.ts, webhook-step.tsx, custom steps)`, + ``, + `If your change is a legitimate infrastructure edit (not a new-provider`, + `addition), update the hash in GUARDED_WIZARD_FILE_HASHES in this test`, + `and include a justification in the commit message.`, + ].join('\n'), + ).toBe(expectedHash); + }); + /** * The explanatory assertion — this is the one that surfaces when * the guard catches something. It always passes on a clean tree; @@ -94,8 +178,14 @@ describe('new-provider-surface (plan 009/5 task 4, spec 009 AC #10)', () => { 'required for a new provider lives in:', ' - src/integrations/pm//', ' - web/src/components/projects/pm-providers//', - ' - A single import line in src/integrations/pm/index.ts', - 'If you need to edit one of the shared surface files above, ', + ' (wizard.ts, state.ts, hooks.ts, auth.ts, webhook-step.tsx, custom steps)', + ' - A single import line in src/integrations/pm/index.ts (backend barrel)', + ' - A single import line in web/src/components/projects/pm-providers/index.ts (frontend barrel)', + 'Shared pm-wizard.tsx, pm-wizard-hooks.ts, and pm-wizard-common-steps.tsx', + 'are intentionally provider-agnostic. The current explicit frontend', + 'exception is pm-wizard-state.ts, which composes provider-owned state', + 'slices from web/src/components/projects/pm-providers//state.ts.', + 'If you need to edit one of the guarded shared surface files above,', 'update this test with the justification and the new expected state.', ].join('\n'); expect(invariant.length).toBeGreaterThan(0); diff --git a/tests/unit/trello/client.test.ts b/tests/unit/trello/client.test.ts index 22c5f8f0f..ba4ef81b9 100644 --- a/tests/unit/trello/client.test.ts +++ b/tests/unit/trello/client.test.ts @@ -355,16 +355,53 @@ describe('trelloClient', () => { }); describe('updateCard', () => { - it('calls updateCard with name and desc', async () => { - mockCards.updateCard.mockResolvedValue({}); + it('sends PUT with name and desc in the JSON body', async () => { + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(JSON.stringify({}), { status: 200 })); await withTrelloCredentials(creds, () => trelloClient.updateCard('card-1', { name: 'New Title', desc: 'New desc' }), ); - expect(mockCards.updateCard).toHaveBeenCalledWith( - expect.objectContaining({ id: 'card-1', name: 'New Title', desc: 'New desc' }), + expect(fetchSpy).toHaveBeenCalledOnce(); + const [url, options] = fetchSpy.mock.calls[0]; + expect(url).toContain('/1/cards/card-1?'); + expect(url).toContain('key=test-key'); + expect(url).toContain('token=test-token'); + expect(options?.method).toBe('PUT'); + expect(options?.headers).toEqual({ 'Content-Type': 'application/json' }); + expect(options?.body).toBe(JSON.stringify({ name: 'New Title', desc: 'New desc' })); + expect(mockCards.updateCard).not.toHaveBeenCalled(); + }); + + it('keeps large descriptions out of the URL', async () => { + const distinctiveText = 'distinctive-long-description-marker'; + const longDescription = `# Long update\n\n${distinctiveText}\n\n${'details '.repeat(2000)}`; + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(JSON.stringify({}), { status: 200 })); + + await withTrelloCredentials(creds, () => + trelloClient.updateCard('card-1', { name: 'New Title', desc: longDescription }), ); + + const [url, options] = fetchSpy.mock.calls[0]; + expect(String(url)).not.toContain('desc'); + expect(String(url)).not.toContain('New Title'); + expect(String(url)).not.toContain(distinctiveText); + expect(options?.body).toBe(JSON.stringify({ name: 'New Title', desc: longDescription })); + }); + + it('serializes an empty desc so card descriptions can be cleared', async () => { + const fetchSpy = vi + .spyOn(globalThis, 'fetch') + .mockResolvedValue(new Response(JSON.stringify({}), { status: 200 })); + + await withTrelloCredentials(creds, () => trelloClient.updateCard('card-1', { desc: '' })); + + const [, options] = fetchSpy.mock.calls[0]; + expect(options?.body).toBe(JSON.stringify({ desc: '' })); }); }); diff --git a/tests/unit/web/jira-webhook-step.test.ts b/tests/unit/web/jira-webhook-step.test.ts index 779627323..0d89c6da5 100644 --- a/tests/unit/web/jira-webhook-step.test.ts +++ b/tests/unit/web/jira-webhook-step.test.ts @@ -11,7 +11,10 @@ import { createElement } from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { describe, expect, it } from 'vitest'; -import { JiraWebhookAdapter } from '../../../web/src/components/projects/pm-providers/jira/webhook-step.js'; +import { + JiraWebhookAdapter, + normalizeJiraActiveWebhooks, +} from '../../../web/src/components/projects/pm-providers/jira/webhook-step.js'; import type { WizardState } from '../../../web/src/components/projects/pm-wizard-state.js'; function makeState(overrides: Partial = {}): WizardState { @@ -37,6 +40,24 @@ function makeProviderHooks(overrides: Record = {}): Record { + it('normalizes JIRA webhook list data from enabled to active', () => { + expect( + normalizeJiraActiveWebhooks({ + jira: [ + { id: 1, url: 'https://hook/jira', enabled: true }, + { id: 'wh-2', url: 'https://hook/jira-2', enabled: false }, + ], + }), + ).toEqual([ + { id: '1', url: 'https://hook/jira', active: true }, + { id: 'wh-2', url: 'https://hook/jira-2', active: false }, + ]); + }); + + it('normalizes missing JIRA webhook list data to an empty active list', () => { + expect(normalizeJiraActiveWebhooks(undefined)).toEqual([]); + }); + it('renders the shared WebhookUrlDisplayStep (URL + copy button)', () => { const html = renderToStaticMarkup( createElement(JiraWebhookAdapter, { @@ -88,6 +109,19 @@ describe('JiraWebhookAdapter', () => { expect(html).toContain('data-action="create-webhook"'); }); + it('renders the loading state while webhooks are loading', () => { + const html = renderToStaticMarkup( + createElement(JiraWebhookAdapter, { + state: makeState(), + dispatch: () => {}, + providerHooks: makeProviderHooks({ webhooksLoading: true }), + }), + ); + expect(html).toContain('data-state="loading"'); + expect(html).toContain('Loading webhooks'); + expect(html).not.toContain('No JIRA webhooks configured'); + }); + it('disables the Create button when callbackBaseUrl is empty', () => { const html = renderToStaticMarkup( createElement(JiraWebhookAdapter, { diff --git a/tests/unit/web/pm-provider-registry.test.ts b/tests/unit/web/pm-provider-registry.test.ts index a6e9267a8..78ca484a2 100644 --- a/tests/unit/web/pm-provider-registry.test.ts +++ b/tests/unit/web/pm-provider-registry.test.ts @@ -20,7 +20,9 @@ function makeStubWizard(id: string): ProviderWizardDefinition { credentialPersistence: [ { envVarKey: 'STUB_API_KEY', stateField: 'linearApiKey', label: 'Stub API Key' }, ], + formatVerificationDisplay: (me) => me.displayName ?? me.name, buildIntegrationConfig: () => ({}), + buildEditState: () => ({ provider: id }), isSetupComplete: () => true, }; } diff --git a/tests/unit/web/pm-wizard-generic-renderer.test.ts b/tests/unit/web/pm-wizard-generic-renderer.test.ts index 730638d0b..ddd4473af 100644 --- a/tests/unit/web/pm-wizard-generic-renderer.test.ts +++ b/tests/unit/web/pm-wizard-generic-renderer.test.ts @@ -1,71 +1,114 @@ -/** - * Unit tests for the tiny registry-check helper that sits in front of the - * per-provider branches in `pm-wizard.tsx`. The helper returns the - * matching step's React element when the provider is registered, or - * `null` when it isn't — so the caller can use `??` to fall back to the - * legacy branch chain. - * - * The goal here is just to prove the seam works; integration-level SSR - * tests for the full wizard come with each provider migration (006/2–4). - */ - import { createElement } from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; -import { beforeEach, describe, expect, it } from 'vitest'; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; import { _resetProviderWizardRegistryForTesting, registerProviderWizard, } from '../../../web/src/components/projects/pm-providers/registry.js'; -import { renderManifestStep } from '../../../web/src/components/projects/pm-providers/render.js'; import type { ProviderWizardDefinition } from '../../../web/src/components/projects/pm-providers/types.js'; +import type { + buildProviderSwitchConfirmationMessage as BuildConfirmationMessage, + buildEditStateFromProviderWizard as BuildEditStateFromProviderWizard, + ProviderPicker as ProviderPickerComponent, +} from '../../../web/src/components/projects/pm-wizard.js'; +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; + +let ProviderPicker: typeof ProviderPickerComponent; +let buildProviderSwitchConfirmationMessage: typeof BuildConfirmationMessage; +let buildEditStateFromProviderWizard: typeof BuildEditStateFromProviderWizard; function StubStep({ state }: { state: { provider: string } }) { - return createElement('div', { 'data-testid': 'stub' }, `stub-${state.provider}`); + return createElement( + 'div', + { 'data-testid': `${state.provider}-stub-step` }, + `stub-${state.provider}`, + ); } -function makeStubWizard(id: string): ProviderWizardDefinition { +function makeStubWizard(id: string, label: string): ProviderWizardDefinition { return { id, - label: id, + label, steps: [ { id: 'credentials', title: 'Credentials', Component: StubStep, isComplete: () => true }, - { id: 'container', title: 'Container', Component: StubStep, isComplete: () => true }, - { id: 'fields', title: 'Field mappings', Component: StubStep, isComplete: () => true }, ], auth: { rawCredentials: [{ role: 'api_key', stateField: 'linearApiKey' }], storedCredentials: { fallbackWhenStateFieldEmpty: 'linearApiKey' }, missingCredentialsMessage: 'Missing credentials', }, + formatVerificationDisplay: (me) => me.displayName ?? me.name, credentialPersistence: [ { envVarKey: 'STUB_API_KEY', stateField: 'linearApiKey', label: 'Stub API Key' }, ], buildIntegrationConfig: () => ({}), + buildEditState: (_initialConfig, configuredKeys) => ({ + provider: id, + hasStoredCredentials: configuredKeys.has('STUB_API_KEY'), + }), isSetupComplete: () => true, }; } -describe('renderManifestStep', () => { +function renderProviderPicker() { + return renderToStaticMarkup( + createElement(ProviderPicker, { + state: createInitialState(), + dispatch: vi.fn(), + advanceToStep: vi.fn(), + }), + ); +} + +describe('PMWizard provider picker', () => { + beforeAll(async () => { + ({ ProviderPicker, buildProviderSwitchConfirmationMessage, buildEditStateFromProviderWizard } = + await import('../../../web/src/components/projects/pm-wizard.js')); + }); + beforeEach(() => { _resetProviderWizardRegistryForTesting(); + registerProviderWizard(makeStubWizard('trello', 'Trello')); + }); + + afterEach(() => { + _resetProviderWizardRegistryForTesting(); + }); + + afterAll(() => { + _resetProviderWizardRegistryForTesting(); + }); + + it('renders every provider returned by the provider wizard registry', () => { + registerProviderWizard(makeStubWizard('acme', 'Acme PM')); + + const html = renderProviderPicker(); + + expect(html).toContain('>Trello'); + expect(html).toContain('>Acme PM'); }); - it('renders the manifest step component when the provider is registered', () => { - registerProviderWizard(makeStubWizard('alpha')); - const element = renderManifestStep('alpha', 0, { provider: 'alpha' } as never, () => {}); - expect(element).not.toBeNull(); - const html = renderToStaticMarkup(element as React.ReactElement); - expect(html).toContain('data-testid="stub"'); - expect(html).toContain('stub-alpha'); + it('builds provider switch confirmation copy from provider definition labels', () => { + expect(buildProviderSwitchConfirmationMessage('Trello', 'Acme PM')).toBe( + "Switch PM provider from Trello to Acme PM?\n\nYou'll need to re-enter credentials and re-map fields for Acme PM. The old provider's credentials will be deleted when you save.", + ); + }); + + it('builds edit state through the registered provider wizard definition', () => { + registerProviderWizard(makeStubWizard('acme', 'Acme PM')); + + const result = buildEditStateFromProviderWizard( + 'acme', + { ignored: true }, + new Set(['STUB_API_KEY']), + ); + + expect(result).toEqual({ provider: 'acme', hasStoredCredentials: true }); }); - it('returns null when the provider is not registered (caller falls back to legacy)', () => { - const element = renderManifestStep( - 'unregistered', - 0, - { provider: 'unregistered' } as never, - () => {}, + it('throws explicitly when edit state is requested for an unknown provider', () => { + expect(() => buildEditStateFromProviderWizard('unknown', {}, new Set())).toThrowError( + 'No PM provider wizard registered for unknown', ); - expect(element).toBeNull(); }); }); diff --git a/tests/unit/web/pm-wizard-hooks-architecture.test.ts b/tests/unit/web/pm-wizard-hooks-architecture.test.ts new file mode 100644 index 000000000..edb2aa300 --- /dev/null +++ b/tests/unit/web/pm-wizard-hooks-architecture.test.ts @@ -0,0 +1,86 @@ +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); +const REPO_ROOT = resolve(__dirname, '..', '..', '..'); +const PM_WIZARD_HOOKS_PATH = resolve(REPO_ROOT, 'web/src/components/projects/pm-wizard-hooks.ts'); + +const PROVIDER_SPECIFIC_HOOK_EXPORTS = [ + 'useTrelloDiscovery', + 'useTrelloLabelCreation', + 'useTrelloCustomFieldCreation', + 'useJiraDiscovery', + 'useJiraLabelCreation', + 'useJiraCustomFieldCreation', + 'useLinearDiscovery', + 'useLinearLabelCreation', + 'useLinearCustomFieldCreation', +] as const; + +function readSharedHooks(): string { + return readFileSync(PM_WIZARD_HOOKS_PATH, 'utf8'); +} + +describe('pm-wizard-hooks architecture boundary', () => { + it('does not export provider-specific hook wrappers', () => { + const source = readSharedHooks(); + + for (const hookName of PROVIDER_SPECIFIC_HOOK_EXPORTS) { + expect( + source, + `${hookName} must live under web/src/components/projects/pm-providers//hooks.ts, not pm-wizard-hooks.ts`, + ).not.toMatch(new RegExp(`\\bexport\\s+(?:function|const)\\s+${hookName}\\b`)); + expect(source, `${hookName} must not be re-exported from pm-wizard-hooks.ts`).not.toMatch( + new RegExp(`\\bexport\\s*\\{[^}]*\\b${hookName}\\b[^}]*\\}`), + ); + } + }); + + it('does not import provider-owned modules', () => { + const source = readSharedHooks(); + + expect( + source, + 'pm-wizard-hooks.ts must stay provider-agnostic; import provider-owned hook/auth modules from provider folders instead.', + ).not.toMatch(/from ['"].*\/pm-providers\/(?:trello|jira|linear)\//); + expect( + source, + 'pm-wizard-hooks.ts must stay provider-agnostic; do not import backend provider modules directly.', + ).not.toMatch(/from ['"].*\/src\/integrations\/pm\/(?:trello|jira|linear)\//); + }); + + it('does not contain provider-owned verification or save dispatch maps', () => { + const source = readSharedHooks(); + + expect( + source, + 'SAVE_CONFIGS belongs in provider metadata, not pm-wizard-hooks.ts', + ).not.toContain('SAVE_CONFIGS'); + expect( + source, + 'Verification/save auth must be metadata-driven; do not branch on state.provider in pm-wizard-hooks.ts.', + ).not.toMatch(/\bstate\.provider\s*={2,3}\s*['"](?:trello|jira|linear)['"]/); + expect( + source, + 'Provider-specific verified-as display formatting belongs on ProviderWizardDefinition.formatVerificationDisplay.', + ).not.toMatch(/\bexport\s+function\s+formatVerificationDisplay\b/); + }); + + it('does not hard-code provider ids in shared config type declarations', () => { + const source = readSharedHooks(); + + // LabelCreationConfig and CustomFieldCreationConfig must accept any provider id + // so that new providers can use the shared factories without editing this file. + expect( + source, + "LabelCreationConfig.providerId must be 'string', not a literal union — new providers must not need to edit pm-wizard-hooks.ts", + ).not.toMatch(/providerId:\s*'(?:trello|jira|linear)'\s*\|/); + expect( + source, + "runPerLabelCreations opts.providerId must be 'string', not a literal union", + ).not.toMatch(/providerId:\s*'(?:trello|jira|linear)'\s*\|/); + }); +}); diff --git a/tests/unit/web/pm-wizard-hooks.test.ts b/tests/unit/web/pm-wizard-hooks.test.ts index 019ce45cb..f85e66ffe 100644 --- a/tests/unit/web/pm-wizard-hooks.test.ts +++ b/tests/unit/web/pm-wizard-hooks.test.ts @@ -1,8 +1,7 @@ /** * Unit tests for pure functions extracted in the pm-wizard-hooks refactor: - * - buildProviderAuthArg (generic auth-arg builder for all three providers) * - runPerLabelCreations (batch label creator with per-item error handling) - * - buildTrelloIntegrationConfig / buildJiraIntegrationConfig (pure config builders) + * - provider-owned buildIntegrationConfig implementations */ import { beforeEach, describe, expect, it, vi } from 'vitest'; @@ -13,154 +12,13 @@ import { buildCurrentUserDiscoveryRequest, buildIntegrationUpsertInput, buildPersistedCredentialInputs, - buildProviderAuthArg, buildProviderAuthArgFromMetadata, - formatVerificationDisplay, + buildProviderCustomFieldCreationRequest, + buildProviderLabelCreationRequest, runPerLabelCreations, } from '../../../web/src/components/projects/pm-wizard-hooks.js'; import type { WizardState } from '../../../web/src/components/projects/pm-wizard-state.js'; -import { - buildJiraIntegrationConfig, - buildLinearIntegrationConfig, - buildTrelloIntegrationConfig, - createInitialState, -} from '../../../web/src/components/projects/pm-wizard-state.js'; - -// ============================================================================ -// buildProviderAuthArg -// ============================================================================ - -describe('buildProviderAuthArg', () => { - function trelloState(overrides: Partial = {}): WizardState { - return { ...createInitialState(), provider: 'trello', ...overrides }; - } - function jiraState(overrides: Partial = {}): WizardState { - return { ...createInitialState(), provider: 'jira', ...overrides }; - } - function linearState(overrides: Partial = {}): WizardState { - return { ...createInitialState(), provider: 'linear', ...overrides }; - } - - // ── Edit mode — stored credentials path ────────────────────────────── - it('trello: returns { projectId } in edit mode when stored creds and no raw key', () => { - const state = trelloState({ - isEditing: true, - hasStoredCredentials: true, - trelloApiKey: '', - trelloToken: '', - }); - expect(buildProviderAuthArg(state, 'proj-1')).toEqual({ projectId: 'proj-1' }); - }); - - it('jira: returns { projectId } in edit mode when stored creds and no raw token', () => { - const state = jiraState({ - isEditing: true, - hasStoredCredentials: true, - jiraApiToken: '', - jiraEmail: '', - }); - expect(buildProviderAuthArg(state, 'proj-jira')).toEqual({ projectId: 'proj-jira' }); - }); - - it('linear: returns { projectId } in edit mode when stored creds and no raw key', () => { - const state = linearState({ - isEditing: true, - hasStoredCredentials: true, - linearApiKey: '', - }); - expect(buildProviderAuthArg(state, 'proj-lin')).toEqual({ projectId: 'proj-lin' }); - }); - - // ── Fresh setup — credentials path ────────────────────────────────── - it('trello: returns credentials when api_key and token present (fresh setup)', () => { - const state = trelloState({ trelloApiKey: 'key-abc', trelloToken: 'tok-xyz' }); - expect(buildProviderAuthArg(state, 'proj-1')).toEqual({ - credentials: { api_key: 'key-abc', token: 'tok-xyz' }, - }); - }); - - it('jira: returns credentials when email + api_token + base_url present (fresh setup)', () => { - const state = jiraState({ - jiraEmail: 'user@example.com', - jiraApiToken: 'jira-tok', - jiraBaseUrl: 'https://example.atlassian.net', - }); - expect(buildProviderAuthArg(state, 'proj-j')).toEqual({ - credentials: { - email: 'user@example.com', - api_token: 'jira-tok', - base_url: 'https://example.atlassian.net', - }, - }); - }); - - it('linear: returns credentials when api_key present (fresh setup)', () => { - const state = linearState({ linearApiKey: 'lin_abc' }); - expect(buildProviderAuthArg(state, 'proj-l')).toEqual({ - credentials: { api_key: 'lin_abc' }, - }); - }); - - // ── Edit mode — user re-typed key → use fresh credentials ─────────── - it('trello: uses fresh credentials when user re-typed api_key in edit mode', () => { - const state = trelloState({ - isEditing: true, - hasStoredCredentials: true, - trelloApiKey: 'new-key', - trelloToken: 'new-tok', - }); - expect(buildProviderAuthArg(state, 'proj-1')).toEqual({ - credentials: { api_key: 'new-key', token: 'new-tok' }, - }); - }); - - it('linear: uses fresh credentials when user re-typed api_key in edit mode', () => { - const state = linearState({ - isEditing: true, - hasStoredCredentials: true, - linearApiKey: 'lin_fresh', - }); - expect(buildProviderAuthArg(state, 'proj-l')).toEqual({ - credentials: { api_key: 'lin_fresh' }, - }); - }); - - // ── Error cases ────────────────────────────────────────────────────── - it('trello: throws when no api_key in fresh mode', () => { - const state = trelloState({ trelloToken: 'tok' }); - expect(() => buildProviderAuthArg(state, 'proj-1')).toThrow( - 'Enter both credentials before verifying', - ); - }); - - it('trello: throws when no token in fresh mode', () => { - const state = trelloState({ trelloApiKey: 'key' }); - expect(() => buildProviderAuthArg(state, 'proj-1')).toThrow( - 'Enter both credentials before verifying', - ); - }); - - it('jira: throws when no email in fresh mode', () => { - const state = jiraState({ jiraApiToken: 'tok', jiraBaseUrl: 'https://x.atlassian.net' }); - expect(() => buildProviderAuthArg(state, 'proj-j')).toThrow( - 'Enter both credentials before verifying', - ); - }); - - it('jira: throws when no api_token in fresh mode', () => { - const state = jiraState({ jiraEmail: 'u@x.com', jiraBaseUrl: 'https://x.atlassian.net' }); - expect(() => buildProviderAuthArg(state, 'proj-j')).toThrow( - 'Enter both credentials before verifying', - ); - }); - - it('linear: throws when no api_key in fresh mode', () => { - const state = linearState({ linearApiKey: '' }); - expect(() => buildProviderAuthArg(state, 'proj-l')).toThrow( - 'Enter your API key before verifying', - ); - }); -}); +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; // ============================================================================ // Provider-owned credential metadata @@ -358,24 +216,235 @@ describe('metadata-driven verification request', () => { it('preserves provider-specific verified-as display formatting', () => { expect( - formatVerificationDisplay('trello', { id: '1', name: 'Full Name', displayName: 'user' }), + trelloProviderWizard.formatVerificationDisplay({ + id: '1', + name: 'Full Name', + displayName: 'user', + }), ).toBe('@user (Full Name)'); expect( - formatVerificationDisplay('jira', { + jiraProviderWizard.formatVerificationDisplay({ id: '2', name: 'Jira User', displayName: 'user@example.com', }), ).toBe('Jira User (user@example.com)'); expect( - formatVerificationDisplay('linear', { id: '3', name: 'Linear User', displayName: 'lin' }), + linearProviderWizard.formatVerificationDisplay({ + id: '3', + name: 'Linear User', + displayName: 'lin', + }), ).toBe('lin'); - expect(formatVerificationDisplay('linear', { id: '4', name: 'Linear User' })).toBe( + expect(linearProviderWizard.formatVerificationDisplay({ id: '4', name: 'Linear User' })).toBe( 'Linear User', ); }); }); +// ============================================================================ +// Metadata-driven mutation requests +// ============================================================================ + +describe('metadata-driven mutation requests', () => { + function trelloState(overrides: Partial = {}): WizardState { + return { ...createInitialState(), provider: 'trello', trelloBoardId: 'board-1', ...overrides }; + } + function jiraState(overrides: Partial = {}): WizardState { + return { ...createInitialState(), provider: 'jira', jiraProjectKey: 'PROJ', ...overrides }; + } + function linearState(overrides: Partial = {}): WizardState { + return { ...createInitialState(), provider: 'linear', linearTeamId: 'team-1', ...overrides }; + } + + it('builds raw credential payloads for provider label creation', () => { + expect( + buildProviderLabelCreationRequest( + { + providerId: trelloProviderWizard.id, + auth: trelloProviderWizard.auth, + getContainerId: (state) => state.trelloBoardId, + containerError: 'Board must be selected before creating a label', + }, + trelloState({ trelloApiKey: 'key', trelloToken: 'token' }), + 'proj-t', + { name: 'cascade-ready', color: 'sky' }, + ), + ).toEqual({ + providerId: 'trello', + containerId: 'board-1', + name: 'cascade-ready', + color: 'sky', + credentials: { api_key: 'key', token: 'token' }, + }); + expect( + buildProviderLabelCreationRequest( + { + providerId: linearProviderWizard.id, + auth: linearProviderWizard.auth, + getContainerId: (state) => state.linearTeamId, + containerError: 'Team must be selected before creating a label', + }, + linearState({ linearApiKey: 'lin-key' }), + 'proj-l', + { name: 'cascade-ready', color: '#0284C7' }, + ), + ).toEqual({ + providerId: 'linear', + containerId: 'team-1', + name: 'cascade-ready', + color: '#0284C7', + credentials: { api_key: 'lin-key' }, + }); + }); + + it('builds stored credential fallback payloads for provider label creation', () => { + expect( + buildProviderLabelCreationRequest( + { + providerId: trelloProviderWizard.id, + auth: trelloProviderWizard.auth, + getContainerId: (state) => state.trelloBoardId, + containerError: 'Board must be selected before creating a label', + }, + trelloState({ + isEditing: true, + hasStoredCredentials: true, + trelloApiKey: '', + trelloToken: '', + }), + 'proj-t', + { name: 'cascade-ready', color: 'sky' }, + ), + ).toMatchObject({ providerId: 'trello', projectId: 'proj-t' }); + expect( + buildProviderLabelCreationRequest( + { + providerId: linearProviderWizard.id, + auth: linearProviderWizard.auth, + getContainerId: (state) => state.linearTeamId, + containerError: 'Team must be selected before creating a label', + }, + linearState({ isEditing: true, hasStoredCredentials: true, linearApiKey: '' }), + 'proj-l', + { name: 'cascade-ready', color: '#0284C7' }, + ), + ).toMatchObject({ providerId: 'linear', projectId: 'proj-l' }); + }); + + it('builds raw credential payloads for provider custom-field creation', () => { + expect( + buildProviderCustomFieldCreationRequest( + { + providerId: trelloProviderWizard.id, + auth: trelloProviderWizard.auth, + getContainerId: (state) => state.trelloBoardId, + containerError: 'Board must be selected before creating a custom field', + }, + trelloState({ trelloApiKey: 'key', trelloToken: 'token' }), + 'proj-t', + { name: 'Cost' }, + ), + ).toEqual({ + providerId: 'trello', + containerId: 'board-1', + name: 'Cost', + credentials: { api_key: 'key', token: 'token' }, + }); + expect( + buildProviderCustomFieldCreationRequest( + { + providerId: jiraProviderWizard.id, + auth: jiraProviderWizard.auth, + getContainerId: (state) => state.jiraProjectKey || 'global', + }, + jiraState({ + jiraEmail: 'user@example.com', + jiraApiToken: 'jira-token', + jiraBaseUrl: 'https://example.atlassian.net', + }), + 'proj-j', + { name: 'Cost' }, + ), + ).toEqual({ + providerId: 'jira', + containerId: 'PROJ', + name: 'Cost', + credentials: { + email: 'user@example.com', + api_token: 'jira-token', + base_url: 'https://example.atlassian.net', + }, + }); + }); + + it('builds stored credential fallback payloads for provider custom-field creation', () => { + expect( + buildProviderCustomFieldCreationRequest( + { + providerId: trelloProviderWizard.id, + auth: trelloProviderWizard.auth, + getContainerId: (state) => state.trelloBoardId, + containerError: 'Board must be selected before creating a custom field', + }, + trelloState({ + isEditing: true, + hasStoredCredentials: true, + trelloApiKey: '', + trelloToken: '', + }), + 'proj-t', + { name: 'Cost' }, + ), + ).toMatchObject({ providerId: 'trello', projectId: 'proj-t' }); + expect( + buildProviderCustomFieldCreationRequest( + { + providerId: jiraProviderWizard.id, + auth: jiraProviderWizard.auth, + getContainerId: (state) => state.jiraProjectKey || 'global', + }, + jiraState({ + isEditing: true, + hasStoredCredentials: true, + jiraEmail: '', + jiraApiToken: '', + }), + 'proj-j', + { name: 'Cost' }, + ), + ).toMatchObject({ providerId: 'jira', projectId: 'proj-j' }); + }); + + it('throws metadata missing-credential errors for mutation requests', () => { + expect(() => + buildProviderLabelCreationRequest( + { + providerId: linearProviderWizard.id, + auth: linearProviderWizard.auth, + getContainerId: (state) => state.linearTeamId, + containerError: 'Team must be selected before creating a label', + }, + linearState({ linearApiKey: '' }), + 'proj-l', + { name: 'cascade-ready' }, + ), + ).toThrow('Enter your API key before verifying'); + expect(() => + buildProviderCustomFieldCreationRequest( + { + providerId: jiraProviderWizard.id, + auth: jiraProviderWizard.auth, + getContainerId: (state) => state.jiraProjectKey || 'global', + }, + jiraState({ jiraEmail: 'user@example.com', jiraApiToken: 'tok', jiraBaseUrl: '' }), + 'proj-j', + { name: 'Cost' }, + ), + ).toThrow('Enter both credentials before verifying'); + }); +}); + // ============================================================================ // Metadata-driven save // ============================================================================ @@ -446,10 +515,10 @@ describe('metadata-driven save payloads', () => { }); // ============================================================================ -// buildTrelloIntegrationConfig +// trelloProviderWizard.buildIntegrationConfig // ============================================================================ -describe('buildTrelloIntegrationConfig', () => { +describe('trelloProviderWizard.buildIntegrationConfig', () => { function seed(overrides: Partial = {}): WizardState { return { ...createInitialState(), @@ -462,7 +531,7 @@ describe('buildTrelloIntegrationConfig', () => { } it('produces the expected config shape', () => { - const config = buildTrelloIntegrationConfig(seed()); + const config = trelloProviderWizard.buildIntegrationConfig(seed()); expect(config).toEqual({ boardId: 'board-abc', lists: { todo: 'list-1', done: 'list-2' }, @@ -471,17 +540,19 @@ describe('buildTrelloIntegrationConfig', () => { }); it('includes customFields when trelloCostFieldId is set', () => { - const config = buildTrelloIntegrationConfig(seed({ trelloCostFieldId: 'cf-cost' })); + const config = trelloProviderWizard.buildIntegrationConfig( + seed({ trelloCostFieldId: 'cf-cost' }), + ); expect(config.customFields).toEqual({ cost: 'cf-cost' }); }); it('omits customFields when trelloCostFieldId is empty', () => { - const config = buildTrelloIntegrationConfig(seed({ trelloCostFieldId: '' })); + const config = trelloProviderWizard.buildIntegrationConfig(seed({ trelloCostFieldId: '' })); expect(config).not.toHaveProperty('customFields'); }); it('passes through empty mappings', () => { - const config = buildTrelloIntegrationConfig( + const config = trelloProviderWizard.buildIntegrationConfig( seed({ trelloListMappings: {}, trelloLabelMappings: {} }), ); expect(config.lists).toEqual({}); @@ -490,10 +561,10 @@ describe('buildTrelloIntegrationConfig', () => { }); // ============================================================================ -// buildJiraIntegrationConfig +// jiraProviderWizard.buildIntegrationConfig // ============================================================================ -describe('buildJiraIntegrationConfig', () => { +describe('jiraProviderWizard.buildIntegrationConfig', () => { function seed(overrides: Partial = {}): WizardState { return { ...createInitialState(), @@ -507,7 +578,7 @@ describe('buildJiraIntegrationConfig', () => { } it('produces the expected config shape', () => { - const config = buildJiraIntegrationConfig(seed()); + const config = jiraProviderWizard.buildIntegrationConfig(seed()); expect(config).toEqual({ projectKey: 'PROJ', baseUrl: 'https://example.atlassian.net', @@ -517,39 +588,40 @@ describe('buildJiraIntegrationConfig', () => { }); it('includes issueTypes when jiraIssueTypes non-empty', () => { - const config = buildJiraIntegrationConfig( + const config = jiraProviderWizard.buildIntegrationConfig( seed({ jiraIssueTypes: { task: 'Task', subtask: 'Sub-task' } }), ); expect(config.issueTypes).toEqual({ task: 'Task', subtask: 'Sub-task' }); }); it('omits issueTypes when jiraIssueTypes is empty', () => { - const config = buildJiraIntegrationConfig(seed({ jiraIssueTypes: {} })); + const config = jiraProviderWizard.buildIntegrationConfig(seed({ jiraIssueTypes: {} })); expect(config).not.toHaveProperty('issueTypes'); }); it('omits labels when jiraLabels is empty', () => { - const config = buildJiraIntegrationConfig(seed({ jiraLabels: {} })); + const config = jiraProviderWizard.buildIntegrationConfig(seed({ jiraLabels: {} })); expect(config).not.toHaveProperty('labels'); }); it('includes customFields when jiraCostFieldId set', () => { - const config = buildJiraIntegrationConfig(seed({ jiraCostFieldId: 'customfield_10042' })); + const config = jiraProviderWizard.buildIntegrationConfig( + seed({ jiraCostFieldId: 'customfield_10042' }), + ); expect(config.customFields).toEqual({ cost: 'customfield_10042' }); }); it('omits customFields when jiraCostFieldId is empty', () => { - const config = buildJiraIntegrationConfig(seed({ jiraCostFieldId: '' })); + const config = jiraProviderWizard.buildIntegrationConfig(seed({ jiraCostFieldId: '' })); expect(config).not.toHaveProperty('customFields'); }); }); // ============================================================================ -// buildLinearIntegrationConfig (already tested in pm-wizard-state.test.ts; -// added here for cross-reference completeness) +// linearProviderWizard.buildIntegrationConfig // ============================================================================ -describe('buildLinearIntegrationConfig', () => { +describe('linearProviderWizard.buildIntegrationConfig', () => { function seed(overrides: Partial = {}): WizardState { return { ...createInitialState(), @@ -562,17 +634,17 @@ describe('buildLinearIntegrationConfig', () => { } it('produces the expected config shape', () => { - const config = buildLinearIntegrationConfig(seed()); + const config = linearProviderWizard.buildIntegrationConfig(seed()); expect(config).toEqual({ teamId: 'T1', statuses: { todo: 'S-TD' } }); }); it('includes projectId when linearProjectId is set', () => { - const config = buildLinearIntegrationConfig(seed({ linearProjectId: 'P1' })); + const config = linearProviderWizard.buildIntegrationConfig(seed({ linearProjectId: 'P1' })); expect(config.projectId).toBe('P1'); }); it('omits projectId when linearProjectId is empty', () => { - const config = buildLinearIntegrationConfig(seed({ linearProjectId: '' })); + const config = linearProviderWizard.buildIntegrationConfig(seed({ linearProjectId: '' })); expect(config).not.toHaveProperty('projectId'); }); }); diff --git a/tests/unit/web/pm-wizard-state.test.ts b/tests/unit/web/pm-wizard-state.test.ts index 09d68a4ae..211baaa67 100644 --- a/tests/unit/web/pm-wizard-state.test.ts +++ b/tests/unit/web/pm-wizard-state.test.ts @@ -1,19 +1,32 @@ import { describe, expect, it } from 'vitest'; +import { + createInitialJiraState, + jiraWizardReducer, + INITIAL_JIRA_LABELS as PROVIDER_INITIAL_JIRA_LABELS, + resetJiraProjectState, +} from '../../../web/src/components/projects/pm-providers/jira/state.js'; +import { jiraProviderWizard } from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; +import { + createInitialLinearState, + INITIAL_LINEAR_LABELS, + linearWizardReducer, + resetLinearTeamState, +} from '../../../web/src/components/projects/pm-providers/linear/state.js'; +import { linearProviderWizard } from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; +import { + createInitialTrelloState, + resetTrelloBoardState, + trelloWizardReducer, +} from '../../../web/src/components/projects/pm-providers/trello/state.js'; +import { trelloProviderWizard } from '../../../web/src/components/projects/pm-providers/trello/wizard.js'; import type { WizardAction, WizardState, } from '../../../web/src/components/projects/pm-wizard-state.js'; import { - areCredentialsReady, - buildEditState, - buildLinearIntegrationConfig, createInitialState, - deriveActiveWebhooks, INITIAL_JIRA_LABELS, isStep1Complete, - isStep2Complete, - isStep3Complete, - isStep4Complete, shouldUseStoredCredentials, wizardReducer, } from '../../../web/src/components/projects/pm-wizard-state.js'; @@ -49,6 +62,133 @@ describe('createInitialState', () => { expect(state.isEditing).toBe(false); expect(state.previousProvider).toBeUndefined(); }); + + it('composes provider-owned initial state slices', () => { + const state = createInitialState(); + expect(state).toMatchObject({ + ...createInitialTrelloState(), + ...createInitialJiraState(), + ...createInitialLinearState(), + }); + }); +}); + +// ============================================================================ +// Provider state slices +// ============================================================================ + +describe('provider state slices', () => { + it('owns the Trello initial state defaults', () => { + expect(createInitialTrelloState()).toEqual({ + trelloApiKey: '', + trelloToken: '', + trelloBoardId: '', + trelloBoards: [], + trelloBoardDetails: null, + trelloListMappings: {}, + trelloLabelMappings: {}, + trelloCostFieldId: '', + }); + }); + + it('owns the JIRA initial state defaults', () => { + expect(createInitialJiraState()).toEqual({ + jiraEmail: '', + jiraApiToken: '', + jiraBaseUrl: '', + jiraProjectKey: '', + jiraProjects: [], + jiraProjectDetails: null, + jiraStatusMappings: {}, + jiraIssueTypes: {}, + jiraLabels: PROVIDER_INITIAL_JIRA_LABELS, + jiraCostFieldId: '', + }); + }); + + it('owns the Linear initial state defaults', () => { + expect(createInitialLinearState()).toEqual({ + linearApiKey: '', + linearTeamId: '', + linearTeams: [], + linearProjectId: '', + linearProjects: [], + linearTeamDetails: null, + linearStatusMappings: {}, + linearLabels: INITIAL_LINEAR_LABELS, + }); + }); + + it('captures Trello board-change reset behavior', () => { + expect(resetTrelloBoardState('board-2')).toEqual({ + trelloBoardId: 'board-2', + trelloBoardDetails: null, + trelloListMappings: {}, + trelloLabelMappings: {}, + trelloCostFieldId: '', + }); + }); + + it('captures JIRA project-change reset behavior', () => { + expect(resetJiraProjectState('NEXT')).toEqual({ + jiraProjectKey: 'NEXT', + jiraProjectDetails: null, + jiraStatusMappings: {}, + jiraIssueTypes: {}, + jiraCostFieldId: '', + }); + }); + + it('captures Linear team-change reset behavior, including project scope', () => { + expect(resetLinearTeamState('team-2')).toEqual({ + linearTeamId: 'team-2', + linearTeamDetails: null, + linearStatusMappings: {}, + linearProjectId: '', + linearProjects: [], + }); + }); + + it('handles Trello actions in the Trello state reducer', () => { + const state = { + ...createInitialState(), + trelloListMappings: { backlog: 'list-1' }, + }; + const next = trelloWizardReducer(state, { + type: 'SET_TRELLO_LIST_MAPPING', + key: 'todo', + value: 'list-2', + }); + expect(next.trelloListMappings).toEqual({ backlog: 'list-1', todo: 'list-2' }); + }); + + it('handles JIRA actions in the JIRA state reducer', () => { + const state = { + ...createInitialState(), + jiraIssueTypes: { task: 'Task' }, + }; + const next = jiraWizardReducer(state, { + type: 'SET_JIRA_ISSUE_TYPE', + key: 'subtask', + value: 'Sub-task', + }); + expect(next.jiraIssueTypes).toEqual({ task: 'Task', subtask: 'Sub-task' }); + }); + + it('handles Linear team reset in the Linear state reducer', () => { + const state = { + ...createInitialState(), + linearTeamId: 'team-1', + linearProjectId: 'project-1', + linearProjects: [{ id: 'project-1', name: 'Project 1', icon: null, color: null }], + linearStatusMappings: { todo: 'state-1' }, + }; + const next = linearWizardReducer(state, { type: 'SET_LINEAR_TEAM_ID', id: 'team-2' }); + expect(next.linearTeamId).toBe('team-2'); + expect(next.linearProjectId).toBe(''); + expect(next.linearProjects).toEqual([]); + expect(next.linearStatusMappings).toEqual({}); + }); }); // ============================================================================ @@ -459,167 +599,6 @@ describe('isStep1Complete', () => { }); }); -describe('isStep2Complete', () => { - it('returns true in edit mode when hasStoredCredentials is true (no raw creds needed)', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - isEditing: true, - hasStoredCredentials: true, - }; - expect(isStep2Complete(state)).toBe(true); - }); - - it('returns false when trello credentials missing', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - verificationResult: { provider: 'trello' as const, display: '@user' }, - }; - expect(isStep2Complete(state)).toBe(false); - }); - - it('returns false when trello creds present but no verification', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - trelloApiKey: 'my-api-key', - trelloToken: 'my-token', - }; - expect(isStep2Complete(state)).toBe(false); - }); - - it('returns true when trello creds present and verified', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - trelloApiKey: 'my-api-key', - trelloToken: 'my-token', - verificationResult: { provider: 'trello' as const, display: '@user (User)' }, - }; - expect(isStep2Complete(state)).toBe(true); - }); - - it('returns false when jira baseUrl missing even with creds', () => { - const state = { - ...createInitialState(), - provider: 'jira' as const, - jiraEmail: 'user@example.com', - jiraApiToken: 'my-token', - jiraBaseUrl: '', - verificationResult: { provider: 'jira' as const, display: 'User' }, - }; - expect(isStep2Complete(state)).toBe(false); - }); - - it('returns true when jira creds and baseUrl present and verified', () => { - const state = { - ...createInitialState(), - provider: 'jira' as const, - jiraEmail: 'user@example.com', - jiraApiToken: 'my-token', - jiraBaseUrl: 'https://myorg.atlassian.net', - verificationResult: { provider: 'jira' as const, display: 'User (user@example.com)' }, - }; - expect(isStep2Complete(state)).toBe(true); - }); -}); - -describe('isStep3Complete', () => { - it('returns true for trello when boardId set', () => { - const state = { ...createInitialState(), provider: 'trello' as const, trelloBoardId: 'b1' }; - expect(isStep3Complete(state)).toBe(true); - }); - - it('returns false for trello when boardId empty', () => { - const state = { ...createInitialState(), provider: 'trello' as const }; - expect(isStep3Complete(state)).toBe(false); - }); - - it('returns true for jira when projectKey set', () => { - const state = { ...createInitialState(), provider: 'jira' as const, jiraProjectKey: 'PROJ' }; - expect(isStep3Complete(state)).toBe(true); - }); - - it('returns false for jira when projectKey empty', () => { - const state = { ...createInitialState(), provider: 'jira' as const }; - expect(isStep3Complete(state)).toBe(false); - }); -}); - -describe('isStep4Complete', () => { - it('returns true for trello when any list mapping set', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - trelloListMappings: { todo: 'list-1' }, - }; - expect(isStep4Complete(state)).toBe(true); - }); - - it('returns false for trello when no list mappings', () => { - const state = { ...createInitialState(), provider: 'trello' as const }; - expect(isStep4Complete(state)).toBe(false); - }); - - it('returns true for jira when any status mapping set', () => { - const state = { - ...createInitialState(), - provider: 'jira' as const, - jiraStatusMappings: { todo: 'To Do' }, - }; - expect(isStep4Complete(state)).toBe(true); - }); - - it('returns false for jira when no status mappings', () => { - const state = { ...createInitialState(), provider: 'jira' as const }; - expect(isStep4Complete(state)).toBe(false); - }); -}); - -describe('areCredentialsReady', () => { - it('returns true for trello when both credentials set', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - trelloApiKey: 'my-api-key', - trelloToken: 'my-token', - }; - expect(areCredentialsReady(state)).toBe(true); - }); - - it('returns false for trello when one credential missing', () => { - const state = { - ...createInitialState(), - provider: 'trello' as const, - trelloApiKey: 'my-api-key', - }; - expect(areCredentialsReady(state)).toBe(false); - }); - - it('returns true for jira when email, api token, and baseUrl set', () => { - const state = { - ...createInitialState(), - provider: 'jira' as const, - jiraEmail: 'user@example.com', - jiraApiToken: 'my-token', - jiraBaseUrl: 'https://myorg.atlassian.net', - }; - expect(areCredentialsReady(state)).toBe(true); - }); - - it('returns false for jira when baseUrl missing', () => { - const state = { - ...createInitialState(), - provider: 'jira' as const, - jiraEmail: 'user@example.com', - jiraApiToken: 'my-token', - jiraBaseUrl: '', - }; - expect(areCredentialsReady(state)).toBe(false); - }); -}); - describe('shouldUseStoredCredentials', () => { // When editing an existing integration, the form does NOT pre-fill // the API key for security — `hasStoredCredentials` is flipped true @@ -719,10 +698,10 @@ describe('shouldUseStoredCredentials', () => { }); // ============================================================================ -// buildEditState +// ProviderWizardDefinition.buildEditState // ============================================================================ -describe('buildEditState', () => { +describe('provider-owned edit hydration', () => { it('builds trello edit state from config', () => { const config = { boardId: 'board-abc', @@ -730,7 +709,7 @@ describe('buildEditState', () => { labels: { processing: 'label-x' }, customFields: { cost: 'cf-cost-1' }, }; - const result = buildEditState('trello', config, new Set()); + const result = trelloProviderWizard.buildEditState(config, new Set()); expect(result.provider).toBe('trello'); // Raw credential values are NOT pre-populated for security expect(result.trelloApiKey).toBeUndefined(); @@ -743,17 +722,20 @@ describe('buildEditState', () => { it('sets hasStoredCredentials true for trello when both keys present', () => { const config = { boardId: 'board-abc' }; - const result = buildEditState('trello', config, new Set(['TRELLO_API_KEY', 'TRELLO_TOKEN'])); + const result = trelloProviderWizard.buildEditState( + config, + new Set(['TRELLO_API_KEY', 'TRELLO_TOKEN']), + ); expect(result.hasStoredCredentials).toBe(true); }); it('sets hasStoredCredentials false for trello when only one key present', () => { - const result = buildEditState('trello', {}, new Set(['TRELLO_API_KEY'])); + const result = trelloProviderWizard.buildEditState({}, new Set(['TRELLO_API_KEY'])); expect(result.hasStoredCredentials).toBe(false); }); it('sets hasStoredCredentials false for trello when no keys present', () => { - const result = buildEditState('trello', {}, new Set()); + const result = trelloProviderWizard.buildEditState({}, new Set()); expect(result.hasStoredCredentials).toBe(false); }); @@ -766,7 +748,7 @@ describe('buildEditState', () => { labels: { processing: 'cascade-processing' }, customFields: { cost: 'customfield_10042' }, }; - const result = buildEditState('jira', config, new Set()); + const result = jiraProviderWizard.buildEditState(config, new Set()); expect(result.provider).toBe('jira'); // Raw credential values are NOT pre-populated for security expect(result.jiraEmail).toBeUndefined(); @@ -780,8 +762,7 @@ describe('buildEditState', () => { }); it('sets hasStoredCredentials true for jira when both keys present', () => { - const result = buildEditState( - 'jira', + const result = jiraProviderWizard.buildEditState( { baseUrl: 'https://example.atlassian.net', projectKey: 'PROJ' }, new Set(['JIRA_EMAIL', 'JIRA_API_TOKEN']), ); @@ -789,23 +770,17 @@ describe('buildEditState', () => { }); it('sets hasStoredCredentials false for jira when only one key present', () => { - const result = buildEditState('jira', {}, new Set(['JIRA_EMAIL'])); + const result = jiraProviderWizard.buildEditState({}, new Set(['JIRA_EMAIL'])); expect(result.hasStoredCredentials).toBe(false); }); it('handles missing optional config fields gracefully', () => { const config = { boardId: 'board-1' }; - const result = buildEditState('trello', config, new Set()); + const result = trelloProviderWizard.buildEditState(config, new Set()); expect(result.trelloBoardId).toBe('board-1'); expect(result.trelloListMappings).toBeUndefined(); expect(result.trelloCostFieldId).toBe(''); }); - - it('returns only provider for unknown provider', () => { - const result = buildEditState('unknown', {}, new Set()); - expect(result.provider).toBe('unknown'); - expect(Object.keys(result).length).toBe(1); - }); }); // ============================================================================ @@ -883,8 +858,7 @@ describe('Linear project scope — wizardReducer', () => { describe('Linear project scope — buildEditState hydration', () => { it('hydrates linearProjectId from initialConfig.projectId when present', () => { - const result = buildEditState( - 'linear', + const result = linearProviderWizard.buildEditState( { teamId: 'T1', projectId: 'P1', statuses: {} }, new Set(['LINEAR_API_KEY']), ); @@ -892,8 +866,7 @@ describe('Linear project scope — buildEditState hydration', () => { }); it('leaves linearProjectId unset when initialConfig has no projectId', () => { - const result = buildEditState( - 'linear', + const result = linearProviderWizard.buildEditState( { teamId: 'T1', statuses: {} }, new Set(['LINEAR_API_KEY']), ); @@ -901,7 +874,7 @@ describe('Linear project scope — buildEditState hydration', () => { }); }); -describe('buildLinearIntegrationConfig — save payload', () => { +describe('linearProviderWizard.buildIntegrationConfig — save payload', () => { function seed(overrides: Partial = {}): WizardState { return { ...createInitialState(), @@ -914,13 +887,13 @@ describe('buildLinearIntegrationConfig — save payload', () => { } it('omits projectId when linearProjectId is empty', () => { - const config = buildLinearIntegrationConfig(seed({ linearProjectId: '' })); + const config = linearProviderWizard.buildIntegrationConfig(seed({ linearProjectId: '' })); expect(config).not.toHaveProperty('projectId'); expect(config.teamId).toBe('T1'); }); it('includes projectId when linearProjectId is set', () => { - const config = buildLinearIntegrationConfig(seed({ linearProjectId: 'P1' })); + const config = linearProviderWizard.buildIntegrationConfig(seed({ linearProjectId: 'P1' })); expect(config.projectId).toBe('P1'); expect(config.teamId).toBe('T1'); }); @@ -928,14 +901,14 @@ describe('buildLinearIntegrationConfig — save payload', () => { it('clearing a previously-set projectId yields a config without projectId', () => { // Simulate edit mode: start with projectId set, user clears, we save. const state = seed({ linearProjectId: '' }); // after clear - const config = buildLinearIntegrationConfig(state); + const config = linearProviderWizard.buildIntegrationConfig(state); expect(config).not.toHaveProperty('projectId'); }); it('omits labels when linearLabels is empty; includes when populated', () => { - const bare = buildLinearIntegrationConfig(seed()); + const bare = linearProviderWizard.buildIntegrationConfig(seed()); expect(bare).not.toHaveProperty('labels'); - const withLabels = buildLinearIntegrationConfig( + const withLabels = linearProviderWizard.buildIntegrationConfig( // Linear labels are stored as UUIDs (the Linear API rejects names for // issueUpdate.labelIds). Wizard dropdowns populate from the team's labels. seed({ linearLabels: { processing: '11111111-1111-4111-8111-111111111111' } }), @@ -945,54 +918,3 @@ describe('buildLinearIntegrationConfig — save payload', () => { }); }); }); - -// ============================================================================ -// deriveActiveWebhooks -// ============================================================================ - -describe('deriveActiveWebhooks', () => { - it('maps Trello webhooks via callbackURL + active', () => { - const result = deriveActiveWebhooks('trello', { - trello: [ - { id: 1, callbackURL: 'https://hook/trello', active: true }, - { id: 2, callbackURL: 'https://hook/trello-2', active: false }, - ], - }); - - expect(result).toEqual([ - { id: '1', url: 'https://hook/trello', active: true }, - { id: '2', url: 'https://hook/trello-2', active: false }, - ]); - }); - - it('maps JIRA webhooks via url + enabled (renamed to active)', () => { - const result = deriveActiveWebhooks('jira', { - jira: [{ id: 'abc', url: 'https://hook/jira', enabled: true }], - }); - - expect(result).toEqual([{ id: 'abc', url: 'https://hook/jira', active: true }]); - }); - - it('returns empty array for Linear (manual webhook setup)', () => { - const result = deriveActiveWebhooks('linear', { - trello: [{ id: 1, callbackURL: 'irrelevant', active: true }], - jira: [{ id: 1, url: 'irrelevant', enabled: true }], - }); - - expect(result).toEqual([]); - }); - - it('returns empty array when webhooksData is undefined', () => { - expect(deriveActiveWebhooks('trello', undefined)).toEqual([]); - expect(deriveActiveWebhooks('jira', undefined)).toEqual([]); - expect(deriveActiveWebhooks('linear', undefined)).toEqual([]); - }); - - it('coerces numeric IDs to strings', () => { - const result = deriveActiveWebhooks('trello', { - trello: [{ id: 42, callbackURL: 'u', active: true }], - }); - expect(result[0].id).toBe('42'); - expect(typeof result[0].id).toBe('string'); - }); -}); diff --git a/tests/unit/web/trello-webhook-step.test.ts b/tests/unit/web/trello-webhook-step.test.ts index 73f248cf2..8a56a4fb4 100644 --- a/tests/unit/web/trello-webhook-step.test.ts +++ b/tests/unit/web/trello-webhook-step.test.ts @@ -14,7 +14,10 @@ import { createElement } from 'react'; import { renderToStaticMarkup } from 'react-dom/server'; import { describe, expect, it } from 'vitest'; -import { TrelloWebhookAdapter } from '../../../web/src/components/projects/pm-providers/trello/webhook-step.js'; +import { + normalizeTrelloActiveWebhooks, + TrelloWebhookAdapter, +} from '../../../web/src/components/projects/pm-providers/trello/webhook-step.js'; import type { WizardState } from '../../../web/src/components/projects/pm-wizard-state.js'; function makeState(overrides: Partial = {}): WizardState { @@ -40,6 +43,24 @@ function makeProviderHooks(overrides: Record = {}): Record { + it('normalizes Trello webhook list data from callbackURL to url', () => { + expect( + normalizeTrelloActiveWebhooks({ + trello: [ + { id: 1, callbackURL: 'https://hook/trello', active: true }, + { id: 2, callbackURL: 'https://hook/trello-2', active: false }, + ], + }), + ).toEqual([ + { id: '1', url: 'https://hook/trello', active: true }, + { id: '2', url: 'https://hook/trello-2', active: false }, + ]); + }); + + it('normalizes missing Trello webhook list data to an empty active list', () => { + expect(normalizeTrelloActiveWebhooks(undefined)).toEqual([]); + }); + it('renders the shared WebhookUrlDisplayStep (URL + copy button)', () => { const html = renderToStaticMarkup( createElement(TrelloWebhookAdapter, { @@ -91,6 +112,19 @@ describe('TrelloWebhookAdapter', () => { expect(html).toContain('data-action="create-webhook"'); }); + it('renders the loading state while webhooks are loading', () => { + const html = renderToStaticMarkup( + createElement(TrelloWebhookAdapter, { + state: makeState(), + dispatch: () => {}, + providerHooks: makeProviderHooks({ webhooksLoading: true }), + }), + ); + expect(html).toContain('data-state="loading"'); + expect(html).toContain('Loading webhooks'); + expect(html).not.toContain('No Trello webhooks configured'); + }); + it('disables the Create button when callbackBaseUrl is empty', () => { const html = renderToStaticMarkup( createElement(TrelloWebhookAdapter, { diff --git a/tests/unit/web/wizard-alerts-slot.test.ts b/tests/unit/web/wizard-alerts-slot.test.ts index 484896779..c8f1ad7bc 100644 --- a/tests/unit/web/wizard-alerts-slot.test.ts +++ b/tests/unit/web/wizard-alerts-slot.test.ts @@ -12,21 +12,19 @@ import { describe, expect, it } from 'vitest'; import { JIRA_LABEL_SLOTS, JIRA_STATUS_SLOTS, + jiraProviderWizard, } from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; import { LINEAR_LABEL_SLOTS, LINEAR_STATUS_SLOTS, + linearProviderWizard, } from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; import { TRELLO_LABEL_SLOTS, TRELLO_LIST_SLOTS, + trelloProviderWizard, } from '../../../web/src/components/projects/pm-providers/trello/wizard.js'; -import { - buildJiraIntegrationConfig, - buildLinearIntegrationConfig, - buildTrelloIntegrationConfig, - createInitialState, -} from '../../../web/src/components/projects/pm-wizard-state.js'; +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; // ============================================================================ // 1. Slot-array membership @@ -70,7 +68,7 @@ describe('alerts status slot round-trips through buildIntegrationConfig', () => ...createInitialState(), trelloListMappings: { alerts: 'list-id-alerts-123' }, }; - const config = buildTrelloIntegrationConfig(state); + const config = trelloProviderWizard.buildIntegrationConfig(state); expect((config.lists as Record).alerts).toBe('list-id-alerts-123'); }); @@ -79,7 +77,7 @@ describe('alerts status slot round-trips through buildIntegrationConfig', () => ...createInitialState(), jiraStatusMappings: { alerts: 'Alerts Status' }, }; - const config = buildJiraIntegrationConfig(state); + const config = jiraProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).alerts).toBe('Alerts Status'); }); @@ -88,7 +86,7 @@ describe('alerts status slot round-trips through buildIntegrationConfig', () => ...createInitialState(), linearStatusMappings: { alerts: 'state-uuid-abc' }, }; - const config = buildLinearIntegrationConfig(state); + const config = linearProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).alerts).toBe('state-uuid-abc'); }); }); @@ -99,7 +97,7 @@ describe('cascade-alert label slot round-trips through buildIntegrationConfig', ...createInitialState(), trelloLabelMappings: { 'cascade-alert': 'label-id-789' }, }; - const config = buildTrelloIntegrationConfig(state); + const config = trelloProviderWizard.buildIntegrationConfig(state); expect((config.labels as Record)['cascade-alert']).toBe('label-id-789'); }); @@ -108,7 +106,7 @@ describe('cascade-alert label slot round-trips through buildIntegrationConfig', ...createInitialState(), jiraLabels: { cascadeAlert: 'cascade-alert-label' }, }; - const config = buildJiraIntegrationConfig(state); + const config = jiraProviderWizard.buildIntegrationConfig(state); expect((config.labels as Record).cascadeAlert).toBe('cascade-alert-label'); }); @@ -117,7 +115,7 @@ describe('cascade-alert label slot round-trips through buildIntegrationConfig', ...createInitialState(), linearLabels: { cascadeAlert: 'label-uuid-xyz' }, }; - const config = buildLinearIntegrationConfig(state); + const config = linearProviderWizard.buildIntegrationConfig(state); expect((config.labels as Record).cascadeAlert).toBe('label-uuid-xyz'); }); }); @@ -127,30 +125,30 @@ describe('cascade-alert label slot round-trips through buildIntegrationConfig', // ============================================================================ describe('alerts slot is optional — wizard does not require it', () => { - it('Trello: buildTrelloIntegrationConfig produces no alerts entry when not mapped', () => { + it('Trello: provider buildIntegrationConfig produces no alerts entry when not mapped', () => { const state = { ...createInitialState(), trelloListMappings: { todo: 'list-id-todo' }, }; - const config = buildTrelloIntegrationConfig(state); + const config = trelloProviderWizard.buildIntegrationConfig(state); expect((config.lists as Record).alerts).toBeUndefined(); }); - it('JIRA: buildJiraIntegrationConfig produces no alerts entry when not mapped', () => { + it('JIRA: provider buildIntegrationConfig produces no alerts entry when not mapped', () => { const state = { ...createInitialState(), jiraStatusMappings: { todo: 'To Do' }, }; - const config = buildJiraIntegrationConfig(state); + const config = jiraProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).alerts).toBeUndefined(); }); - it('Linear: buildLinearIntegrationConfig produces no alerts entry when not mapped', () => { + it('Linear: provider buildIntegrationConfig produces no alerts entry when not mapped', () => { const state = { ...createInitialState(), linearStatusMappings: { todo: 'state-uuid-todo' }, }; - const config = buildLinearIntegrationConfig(state); + const config = linearProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).alerts).toBeUndefined(); }); }); diff --git a/tests/unit/web/wizard-friction-slot.test.ts b/tests/unit/web/wizard-friction-slot.test.ts index 5877f44b0..d82099891 100644 --- a/tests/unit/web/wizard-friction-slot.test.ts +++ b/tests/unit/web/wizard-friction-slot.test.ts @@ -9,15 +9,19 @@ */ import { describe, expect, it } from 'vitest'; -import { JIRA_STATUS_SLOTS } from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; -import { LINEAR_STATUS_SLOTS } from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; -import { TRELLO_LIST_SLOTS } from '../../../web/src/components/projects/pm-providers/trello/wizard.js'; import { - buildJiraIntegrationConfig, - buildLinearIntegrationConfig, - buildTrelloIntegrationConfig, - createInitialState, -} from '../../../web/src/components/projects/pm-wizard-state.js'; + JIRA_STATUS_SLOTS, + jiraProviderWizard, +} from '../../../web/src/components/projects/pm-providers/jira/wizard.js'; +import { + LINEAR_STATUS_SLOTS, + linearProviderWizard, +} from '../../../web/src/components/projects/pm-providers/linear/wizard.js'; +import { + TRELLO_LIST_SLOTS, + trelloProviderWizard, +} from '../../../web/src/components/projects/pm-providers/trello/wizard.js'; +import { createInitialState } from '../../../web/src/components/projects/pm-wizard-state.js'; // ============================================================================ // 1. Slot-array membership @@ -47,7 +51,7 @@ describe('friction status slot round-trips through buildIntegrationConfig', () = ...createInitialState(), trelloListMappings: { friction: 'list-id-friction-123' }, }; - const config = buildTrelloIntegrationConfig(state); + const config = trelloProviderWizard.buildIntegrationConfig(state); expect((config.lists as Record).friction).toBe('list-id-friction-123'); }); @@ -56,7 +60,7 @@ describe('friction status slot round-trips through buildIntegrationConfig', () = ...createInitialState(), jiraStatusMappings: { friction: 'Friction Status' }, }; - const config = buildJiraIntegrationConfig(state); + const config = jiraProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).friction).toBe('Friction Status'); }); @@ -65,7 +69,7 @@ describe('friction status slot round-trips through buildIntegrationConfig', () = ...createInitialState(), linearStatusMappings: { friction: 'state-uuid-friction' }, }; - const config = buildLinearIntegrationConfig(state); + const config = linearProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).friction).toBe('state-uuid-friction'); }); }); @@ -75,30 +79,30 @@ describe('friction status slot round-trips through buildIntegrationConfig', () = // ============================================================================ describe('friction slot is optional — wizard does not require it', () => { - it('Trello: buildTrelloIntegrationConfig produces no friction entry when not mapped', () => { + it('Trello: provider buildIntegrationConfig produces no friction entry when not mapped', () => { const state = { ...createInitialState(), trelloListMappings: { todo: 'list-id-todo' }, }; - const config = buildTrelloIntegrationConfig(state); + const config = trelloProviderWizard.buildIntegrationConfig(state); expect((config.lists as Record).friction).toBeUndefined(); }); - it('JIRA: buildJiraIntegrationConfig produces no friction entry when not mapped', () => { + it('JIRA: provider buildIntegrationConfig produces no friction entry when not mapped', () => { const state = { ...createInitialState(), jiraStatusMappings: { todo: 'To Do' }, }; - const config = buildJiraIntegrationConfig(state); + const config = jiraProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).friction).toBeUndefined(); }); - it('Linear: buildLinearIntegrationConfig produces no friction entry when not mapped', () => { + it('Linear: provider buildIntegrationConfig produces no friction entry when not mapped', () => { const state = { ...createInitialState(), linearStatusMappings: { todo: 'state-uuid-todo' }, }; - const config = buildLinearIntegrationConfig(state); + const config = linearProviderWizard.buildIntegrationConfig(state); expect((config.statuses as Record).friction).toBeUndefined(); }); }); diff --git a/web/src/components/projects/pm-providers/index.ts b/web/src/components/projects/pm-providers/index.ts new file mode 100644 index 000000000..78278ef0b --- /dev/null +++ b/web/src/components/projects/pm-providers/index.ts @@ -0,0 +1,24 @@ +/** + * Frontend PM provider barrel — side-effect imports register each provider's + * wizard definition into `providerWizardRegistry` at module load. + * + * This mirrors the backend barrel at `src/integrations/pm/index.ts`. The + * single import of this module in `pm-wizard.tsx` ensures every provider is + * registered before the wizard renders. + * + * Adding a new PM provider? Add exactly one line here: + * import './/index.js'; + * + * Shared orchestration files (`pm-wizard.tsx`, `pm-wizard-hooks.ts`, + * `pm-wizard-common-steps.tsx`) need zero edits after this import. + * The one shared dashboard file that still requires manual edits is + * `pm-wizard-state.ts` — new providers must add their credential fields + * to `WizardState` and the corresponding action types to `WizardAction`. + * Provider-specific edit hydration belongs on the provider's + * `ProviderWizardDefinition.buildEditState`. See step 4 of "Adding a new PM + * provider" in src/integrations/README.md. + */ + +import './trello/index.js'; +import './jira/index.js'; +import './linear/index.js'; diff --git a/web/src/components/projects/pm-providers/jira/auth.ts b/web/src/components/projects/pm-providers/jira/auth.ts new file mode 100644 index 000000000..7767419ec --- /dev/null +++ b/web/src/components/projects/pm-providers/jira/auth.ts @@ -0,0 +1,16 @@ +import type { ProviderAuthMetadata, ProviderCredentialPersistenceMapping } from '../types.js'; + +export const jiraAuthMetadata: ProviderAuthMetadata = { + rawCredentials: [ + { role: 'email', stateField: 'jiraEmail' }, + { role: 'api_token', stateField: 'jiraApiToken' }, + { role: 'base_url', stateField: 'jiraBaseUrl' }, + ], + storedCredentials: { fallbackWhenStateFieldEmpty: 'jiraEmail' }, + missingCredentialsMessage: 'Enter both credentials before verifying', +}; + +export const jiraCredentialPersistence: readonly ProviderCredentialPersistenceMapping[] = [ + { envVarKey: 'JIRA_EMAIL', stateField: 'jiraEmail', label: 'JIRA Email' }, + { envVarKey: 'JIRA_API_TOKEN', stateField: 'jiraApiToken', label: 'JIRA API Token' }, +]; diff --git a/web/src/components/projects/pm-providers/jira/hooks.ts b/web/src/components/projects/pm-providers/jira/hooks.ts index 037712e4d..f2633f9ec 100644 --- a/web/src/components/projects/pm-providers/jira/hooks.ts +++ b/web/src/components/projects/pm-providers/jira/hooks.ts @@ -4,6 +4,7 @@ import { useEffect } from 'react'; import { trpcClient } from '@/lib/trpc.js'; import { useProviderCustomFieldCreation } from '../../pm-wizard-hooks.js'; import type { WizardAction, WizardState } from '../../pm-wizard-state.js'; +import type { ProviderAuthMetadata } from '../types.js'; // ============================================================================ // JIRA Discovery @@ -115,13 +116,16 @@ export function useJiraDiscovery( // ============================================================================ export function useJiraCustomFieldCreation( + providerId: string, + auth: ProviderAuthMetadata, state: WizardState, dispatch: Dispatch, projectId: string, ) { const inner = useProviderCustomFieldCreation( { - providerId: 'jira', + providerId, + auth, // JIRA fields are global; containerId is sent as-is for uniform shape getContainerId: (s) => s.jiraProjectKey || 'global', addCustomField: (f) => ({ diff --git a/web/src/components/projects/pm-providers/jira/state.ts b/web/src/components/projects/pm-providers/jira/state.ts new file mode 100644 index 000000000..08417f28d --- /dev/null +++ b/web/src/components/projects/pm-providers/jira/state.ts @@ -0,0 +1,151 @@ +export interface JiraProjectOption { + key: string; + name: string; +} + +export interface JiraProjectDetails { + statuses: Array<{ name: string; id: string }>; + issueTypes: Array<{ name: string; subtask: boolean }>; + fields: Array<{ id: string; name: string; custom: boolean }>; +} + +export const INITIAL_JIRA_LABELS: Record = { + processing: 'cascade-processing', + processed: 'cascade-processed', + error: 'cascade-error', + readyToProcess: 'cascade-ready', + auto: 'cascade-auto', +}; + +export interface JiraWizardStateSlice { + jiraEmail: string; + jiraApiToken: string; + jiraBaseUrl: string; + jiraProjectKey: string; + jiraProjects: JiraProjectOption[]; + jiraProjectDetails: JiraProjectDetails | null; + jiraStatusMappings: Record; + jiraIssueTypes: Record; + jiraLabels: Record; + jiraCostFieldId: string; +} + +interface VerificationState { + verificationResult: { provider: string; display: string } | null; + verifyError: string | null; +} + +export type JiraWizardAction = + | { type: 'SET_JIRA_EMAIL'; value: string } + | { type: 'SET_JIRA_API_TOKEN'; value: string } + | { type: 'SET_JIRA_BASE_URL'; url: string } + | { type: 'SET_JIRA_PROJECTS'; projects: JiraProjectOption[] } + | { type: 'SET_JIRA_PROJECT_KEY'; key: string } + | { type: 'SET_JIRA_PROJECT_DETAILS'; details: JiraProjectDetails | null } + | { type: 'SET_JIRA_STATUS_MAPPING'; key: string; value: string } + | { type: 'SET_JIRA_ISSUE_TYPE'; key: string; value: string } + | { type: 'SET_JIRA_LABEL'; key: string; value: string } + | { type: 'SET_JIRA_COST_FIELD'; id: string } + | { type: 'ADD_JIRA_PROJECT_CUSTOM_FIELD'; field: { id: string; name: string; custom: boolean } }; + +export function createInitialJiraState(): JiraWizardStateSlice { + return { + jiraEmail: '', + jiraApiToken: '', + jiraBaseUrl: '', + jiraProjectKey: '', + jiraProjects: [], + jiraProjectDetails: null, + jiraStatusMappings: {}, + jiraIssueTypes: {}, + jiraLabels: { ...INITIAL_JIRA_LABELS }, + jiraCostFieldId: '', + }; +} + +export function isJiraWizardAction(action: { type: string }): action is JiraWizardAction { + return action.type.includes('JIRA'); +} + +export function jiraWizardReducer( + state: T, + action: JiraWizardAction, +): T { + switch (action.type) { + case 'SET_JIRA_EMAIL': + return { + ...state, + jiraEmail: action.value, + verificationResult: null, + verifyError: null, + }; + case 'SET_JIRA_API_TOKEN': + return { + ...state, + jiraApiToken: action.value, + verificationResult: null, + verifyError: null, + }; + case 'SET_JIRA_BASE_URL': + return { + ...state, + jiraBaseUrl: action.url, + verificationResult: null, + verifyError: null, + }; + case 'SET_JIRA_PROJECTS': + return { ...state, jiraProjects: action.projects }; + case 'SET_JIRA_PROJECT_KEY': + return { + ...state, + ...resetJiraProjectState(action.key), + }; + case 'SET_JIRA_PROJECT_DETAILS': + return { ...state, jiraProjectDetails: action.details }; + case 'SET_JIRA_STATUS_MAPPING': + return { + ...state, + jiraStatusMappings: { ...state.jiraStatusMappings, [action.key]: action.value }, + }; + case 'SET_JIRA_ISSUE_TYPE': + return { + ...state, + jiraIssueTypes: { ...state.jiraIssueTypes, [action.key]: action.value }, + }; + case 'SET_JIRA_LABEL': + return { + ...state, + jiraLabels: { ...state.jiraLabels, [action.key]: action.value }, + }; + case 'SET_JIRA_COST_FIELD': + return { ...state, jiraCostFieldId: action.id }; + case 'ADD_JIRA_PROJECT_CUSTOM_FIELD': + if (!state.jiraProjectDetails) return state; + return { + ...state, + jiraProjectDetails: { + ...state.jiraProjectDetails, + fields: [...state.jiraProjectDetails.fields, action.field], + }, + }; + } +} + +export function resetJiraProjectState( + jiraProjectKey: string, +): Pick< + JiraWizardStateSlice, + | 'jiraProjectKey' + | 'jiraProjectDetails' + | 'jiraStatusMappings' + | 'jiraIssueTypes' + | 'jiraCostFieldId' +> { + return { + jiraProjectKey, + jiraProjectDetails: null, + jiraStatusMappings: {}, + jiraIssueTypes: {}, + jiraCostFieldId: '', + }; +} diff --git a/web/src/components/projects/pm-providers/jira/webhook-step.tsx b/web/src/components/projects/pm-providers/jira/webhook-step.tsx index bf560f372..df7abb5cf 100644 --- a/web/src/components/projects/pm-providers/jira/webhook-step.tsx +++ b/web/src/components/projects/pm-providers/jira/webhook-step.tsx @@ -20,12 +20,30 @@ import type { DataProps } from '@/lib/data-props.js'; import { WebhookUrlDisplayStep } from '../steps/webhook-url-display.js'; import type { ProviderWizardStepProps } from '../types.js'; -interface ActiveWebhook { +export interface ActiveWebhook { readonly id: string; readonly url: string; readonly active: boolean; } +export interface JiraWebhookListData { + readonly jira?: ReadonlyArray<{ + readonly id: string | number; + readonly url: string; + readonly enabled: boolean; + }>; +} + +export function normalizeJiraActiveWebhooks( + webhooksData: JiraWebhookListData | undefined, +): ActiveWebhook[] { + return (webhooksData?.jira ?? []).map((webhook) => ({ + id: String(webhook.id), + url: webhook.url, + active: webhook.enabled, + })); +} + interface JiraWebhookProviderHooks { readonly webhookUrl: string; readonly callbackBaseUrl: string; diff --git a/web/src/components/projects/pm-providers/jira/wizard.ts b/web/src/components/projects/pm-providers/jira/wizard.ts index 5dc94ebae..92ffb4a38 100644 --- a/web/src/components/projects/pm-providers/jira/wizard.ts +++ b/web/src/components/projects/pm-providers/jira/wizard.ts @@ -20,16 +20,16 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; import type { ReactElement } from 'react'; import { API_URL } from '@/lib/api.js'; import { trpc, trpcClient } from '@/lib/trpc.js'; -import { deriveActiveWebhooks } from '../../pm-wizard-state.js'; import { ContainerPickStep } from '../steps/container-pick.js'; import { CredentialsStep } from '../steps/credentials.js'; import { CustomFieldMappingStep } from '../steps/custom-field-mapping.js'; import { LabelMappingStep } from '../steps/label-mapping.js'; import { StatusMappingStep } from '../steps/status-mapping.js'; import type { ProviderWizardDefinition, ProviderWizardStepProps } from '../types.js'; +import { jiraAuthMetadata, jiraCredentialPersistence } from './auth.js'; import { useJiraCustomFieldCreation, useJiraDiscovery } from './hooks.js'; import { IssueTypeMappingStep } from './issue-type-step.js'; -import { JiraWebhookAdapter } from './webhook-step.js'; +import { JiraWebhookAdapter, normalizeJiraActiveWebhooks } from './webhook-step.js'; // CASCADE stage keys that map to JIRA statuses (name-based, not id-based // — JIRA statuses are configured per project, name is the stable identity). @@ -248,19 +248,9 @@ function JiraIssueTypeAdapter({ export const jiraProviderWizard: ProviderWizardDefinition = { id: 'jira', label: 'JIRA', - auth: { - rawCredentials: [ - { role: 'email', stateField: 'jiraEmail' }, - { role: 'api_token', stateField: 'jiraApiToken' }, - { role: 'base_url', stateField: 'jiraBaseUrl' }, - ], - storedCredentials: { fallbackWhenStateFieldEmpty: 'jiraEmail' }, - missingCredentialsMessage: 'Enter both credentials before verifying', - }, - credentialPersistence: [ - { envVarKey: 'JIRA_EMAIL', stateField: 'jiraEmail', label: 'JIRA Email' }, - { envVarKey: 'JIRA_API_TOKEN', stateField: 'jiraApiToken', label: 'JIRA API Token' }, - ], + auth: jiraAuthMetadata, + formatVerificationDisplay: (me) => (me.displayName ? `${me.name} (${me.displayName})` : me.name), + credentialPersistence: jiraCredentialPersistence, steps: [ { @@ -316,15 +306,40 @@ export const jiraProviderWizard: ProviderWizardDefinition = { ...(state.jiraCostFieldId ? { customFields: { cost: state.jiraCostFieldId } } : {}), }), + buildEditState: (initialConfig, configuredKeys) => { + const statuses = initialConfig.statuses as Record | undefined; + const issueTypes = initialConfig.issueTypes as Record | undefined; + const labels = initialConfig.labels as Record | undefined; + + return { + provider: 'jira', + jiraBaseUrl: (initialConfig.baseUrl as string) ?? '', + jiraProjectKey: (initialConfig.projectKey as string) ?? '', + ...(statuses ? { jiraStatusMappings: statuses } : {}), + ...(issueTypes ? { jiraIssueTypes: issueTypes } : {}), + ...(labels ? { jiraLabels: labels } : {}), + jiraCostFieldId: + (initialConfig.customFields as Record | undefined)?.cost ?? '', + hasStoredCredentials: + configuredKeys.has('JIRA_EMAIL') && configuredKeys.has('JIRA_API_TOKEN'), + }; + }, + isSetupComplete: (state) => { if (!state.jiraProjectKey) return false; if (Object.keys(state.jiraStatusMappings).length === 0) return false; return isCredentialsComplete(state); }, - useProviderHooks: ({ state, dispatch, projectId, advanceToStep }) => { + useProviderHooks: ({ providerId, auth, state, dispatch, projectId, advanceToStep }) => { const discovery = useJiraDiscovery(state, dispatch, advanceToStep, projectId ?? ''); - const customField = useJiraCustomFieldCreation(state, dispatch, projectId ?? ''); + const customField = useJiraCustomFieldCreation( + providerId, + auth, + state, + dispatch, + projectId ?? '', + ); const queryClient = useQueryClient(); const onCreateCustomField = (_slotKey: string, name: string) => { @@ -342,11 +357,7 @@ export const jiraProviderWizard: ProviderWizardDefinition = { (typeof window !== 'undefined' ? window.location.origin.replace(':5173', ':3000') : ''); const webhooksQuery = useQuery(trpc.webhooks.list.queryOptions({ projectId: projectId ?? '' })); - const activeJiraWebhooks = deriveActiveWebhooks('jira', webhooksQuery.data) as Array<{ - id: string; - url: string; - active: boolean; - }>; + const activeJiraWebhooks = normalizeJiraActiveWebhooks(webhooksQuery.data); const createWebhookMutation = useMutation({ mutationFn: () => diff --git a/web/src/components/projects/pm-providers/linear/auth.ts b/web/src/components/projects/pm-providers/linear/auth.ts new file mode 100644 index 000000000..5524c0ae1 --- /dev/null +++ b/web/src/components/projects/pm-providers/linear/auth.ts @@ -0,0 +1,11 @@ +import type { ProviderAuthMetadata, ProviderCredentialPersistenceMapping } from '../types.js'; + +export const linearAuthMetadata: ProviderAuthMetadata = { + rawCredentials: [{ role: 'api_key', stateField: 'linearApiKey' }], + storedCredentials: { fallbackWhenStateFieldEmpty: 'linearApiKey' }, + missingCredentialsMessage: 'Enter your API key before verifying', +}; + +export const linearCredentialPersistence: readonly ProviderCredentialPersistenceMapping[] = [ + { envVarKey: 'LINEAR_API_KEY', stateField: 'linearApiKey', label: 'Linear API Key' }, +]; diff --git a/web/src/components/projects/pm-providers/linear/hooks.ts b/web/src/components/projects/pm-providers/linear/hooks.ts index 5644fcd3d..899873b37 100644 --- a/web/src/components/projects/pm-providers/linear/hooks.ts +++ b/web/src/components/projects/pm-providers/linear/hooks.ts @@ -10,6 +10,7 @@ import type { WizardAction, WizardState, } from '../../pm-wizard-state.js'; +import type { ProviderAuthMetadata } from '../types.js'; // ============================================================================ // Linear Discovery @@ -155,13 +156,16 @@ export function useLinearDiscovery( // ============================================================================ export function useLinearLabelCreation( + providerId: string, + auth: ProviderAuthMetadata, state: WizardState, dispatch: Dispatch, projectId: string, ) { return useProviderLabelCreation( { - providerId: 'linear', + providerId, + auth, getContainerId: (s) => s.linearTeamId, containerError: 'Team must be selected before creating a label', addLabel: (label) => ({ type: 'ADD_LINEAR_TEAM_LABEL', label }), diff --git a/web/src/components/projects/pm-providers/linear/state.ts b/web/src/components/projects/pm-providers/linear/state.ts new file mode 100644 index 000000000..a81ea9981 --- /dev/null +++ b/web/src/components/projects/pm-providers/linear/state.ts @@ -0,0 +1,135 @@ +export interface LinearTeamOption { + id: string; + name: string; + key: string; +} + +export interface LinearProjectOption { + id: string; + name: string; + icon: string | null; + color: string | null; +} + +export interface LinearTeamDetails { + states: Array<{ id: string; name: string; type: string }>; + labels: Array<{ id: string; name: string; color: string }>; +} + +/** + * Linear label mappings store workflow-label **UUIDs**, not names, because + * Linear's GraphQL API rejects names for issueUpdate.labelIds. The wizard + * populates these from the team's existing labels or via the create-label + * button. Initial state is therefore empty — operators pick or create. + */ +export const INITIAL_LINEAR_LABELS: Record = {}; + +export interface LinearWizardStateSlice { + linearApiKey: string; + linearTeamId: string; + linearTeams: LinearTeamOption[]; + linearProjectId: string; + linearProjects: LinearProjectOption[]; + linearTeamDetails: LinearTeamDetails | null; + linearStatusMappings: Record; + linearLabels: Record; +} + +interface VerificationState { + verificationResult: { provider: string; display: string } | null; + verifyError: string | null; +} + +export type LinearWizardAction = + | { type: 'SET_LINEAR_API_KEY'; value: string } + | { type: 'SET_LINEAR_TEAMS'; teams: LinearTeamOption[] } + | { type: 'SET_LINEAR_TEAM_ID'; id: string } + | { type: 'SET_LINEAR_TEAM_DETAILS'; details: LinearTeamDetails | null } + | { type: 'SET_LINEAR_PROJECTS'; projects: LinearProjectOption[] } + | { type: 'SET_LINEAR_PROJECT_ID'; value: string } + | { type: 'SET_LINEAR_STATUS_MAPPING'; key: string; value: string } + | { type: 'SET_LINEAR_LABEL'; key: string; value: string } + | { type: 'ADD_LINEAR_TEAM_LABEL'; label: { id: string; name: string; color: string } }; + +export function createInitialLinearState(): LinearWizardStateSlice { + return { + linearApiKey: '', + linearTeamId: '', + linearTeams: [], + linearProjectId: '', + linearProjects: [], + linearTeamDetails: null, + linearStatusMappings: {}, + linearLabels: { ...INITIAL_LINEAR_LABELS }, + }; +} + +export function isLinearWizardAction(action: { type: string }): action is LinearWizardAction { + return action.type.includes('LINEAR'); +} + +export function linearWizardReducer( + state: T, + action: LinearWizardAction, +): T { + switch (action.type) { + case 'SET_LINEAR_API_KEY': + return { + ...state, + linearApiKey: action.value, + verificationResult: null, + verifyError: null, + }; + case 'SET_LINEAR_TEAMS': + return { ...state, linearTeams: action.teams }; + case 'SET_LINEAR_TEAM_ID': + return { + ...state, + ...resetLinearTeamState(action.id), + }; + case 'SET_LINEAR_PROJECTS': + return { ...state, linearProjects: action.projects }; + case 'SET_LINEAR_PROJECT_ID': + return { ...state, linearProjectId: action.value }; + case 'SET_LINEAR_TEAM_DETAILS': + return { ...state, linearTeamDetails: action.details }; + case 'SET_LINEAR_STATUS_MAPPING': + return { + ...state, + linearStatusMappings: { ...state.linearStatusMappings, [action.key]: action.value }, + }; + case 'SET_LINEAR_LABEL': + return { + ...state, + linearLabels: { ...state.linearLabels, [action.key]: action.value }, + }; + case 'ADD_LINEAR_TEAM_LABEL': + if (!state.linearTeamDetails) return state; + return { + ...state, + linearTeamDetails: { + ...state.linearTeamDetails, + labels: [...state.linearTeamDetails.labels, action.label], + }, + }; + } +} + +export function resetLinearTeamState( + linearTeamId: string, +): Pick< + LinearWizardStateSlice, + | 'linearTeamId' + | 'linearTeamDetails' + | 'linearStatusMappings' + | 'linearProjectId' + | 'linearProjects' +> { + return { + linearTeamId, + linearTeamDetails: null, + linearStatusMappings: {}, + linearProjectId: '', + linearProjects: [], + }; +} diff --git a/web/src/components/projects/pm-providers/linear/wizard.ts b/web/src/components/projects/pm-providers/linear/wizard.ts index 5ba51a418..92983781c 100644 --- a/web/src/components/projects/pm-providers/linear/wizard.ts +++ b/web/src/components/projects/pm-providers/linear/wizard.ts @@ -23,7 +23,6 @@ import { useQuery } from '@tanstack/react-query'; import { type ReactElement, useState } from 'react'; import { API_URL } from '@/lib/api.js'; import { trpc } from '@/lib/trpc.js'; -import { buildLinearIntegrationConfig } from '../../pm-wizard-state.js'; import type { ProjectCredentialMeta } from '../../project-secret-field.js'; import { ContainerPickStep } from '../steps/container-pick.js'; import { CredentialsStep } from '../steps/credentials.js'; @@ -31,6 +30,7 @@ import { LabelMappingStep } from '../steps/label-mapping.js'; import { ProjectScopeStep } from '../steps/project-scope.js'; import { StatusMappingStep } from '../steps/status-mapping.js'; import type { ProviderWizardDefinition, ProviderWizardStepProps } from '../types.js'; +import { linearAuthMetadata, linearCredentialPersistence } from './auth.js'; import { useLinearDiscovery, useLinearLabelCreation } from './hooks.js'; import { LinearWebhookAdapter } from './webhook-step.js'; @@ -231,14 +231,9 @@ function LinearProjectScopeAdapter({ providerHooks }: ProviderWizardStepProps): export const linearProviderWizard: ProviderWizardDefinition = { id: 'linear', label: 'Linear', - auth: { - rawCredentials: [{ role: 'api_key', stateField: 'linearApiKey' }], - storedCredentials: { fallbackWhenStateFieldEmpty: 'linearApiKey' }, - missingCredentialsMessage: 'Enter your API key before verifying', - }, - credentialPersistence: [ - { envVarKey: 'LINEAR_API_KEY', stateField: 'linearApiKey', label: 'Linear API Key' }, - ], + auth: linearAuthMetadata, + formatVerificationDisplay: (me) => me.displayName || me.name, + credentialPersistence: linearCredentialPersistence, steps: [ { @@ -279,7 +274,26 @@ export const linearProviderWizard: ProviderWizardDefinition = { }, ], - buildIntegrationConfig: buildLinearIntegrationConfig, + buildIntegrationConfig: (state) => ({ + teamId: state.linearTeamId, + ...(state.linearProjectId ? { projectId: state.linearProjectId } : {}), + statuses: state.linearStatusMappings, + ...(Object.keys(state.linearLabels).length > 0 ? { labels: state.linearLabels } : {}), + }), + + buildEditState: (initialConfig, configuredKeys) => { + const statuses = initialConfig.statuses as Record | undefined; + const labels = initialConfig.labels as Record | undefined; + + return { + provider: 'linear', + linearTeamId: (initialConfig.teamId as string) ?? '', + linearProjectId: (initialConfig.projectId as string) ?? '', + ...(statuses ? { linearStatusMappings: statuses } : {}), + ...(labels ? { linearLabels: labels } : {}), + hasStoredCredentials: configuredKeys.has('LINEAR_API_KEY'), + }; + }, isSetupComplete: (state) => { if (!state.linearTeamId) return false; @@ -287,9 +301,9 @@ export const linearProviderWizard: ProviderWizardDefinition = { return isCredentialsComplete(state); }, - useProviderHooks: ({ state, dispatch, projectId, advanceToStep }) => { + useProviderHooks: ({ providerId, auth, state, dispatch, projectId, advanceToStep }) => { const discovery = useLinearDiscovery(state, dispatch, advanceToStep, projectId ?? ''); - const labels = useLinearLabelCreation(state, dispatch, projectId ?? ''); + const labels = useLinearLabelCreation(providerId, auth, state, dispatch, projectId ?? ''); // Lift the LINEAR_WEBHOOK_SECRET credential lookup from the parent // wizard (`pm-wizard.tsx`) into the provider hooks so the Linear // webhook step adapter can compose the shared `WebhookUrlDisplayStep` diff --git a/web/src/components/projects/pm-providers/registry.ts b/web/src/components/projects/pm-providers/registry.ts index e352371e7..8bced2fab 100644 --- a/web/src/components/projects/pm-providers/registry.ts +++ b/web/src/components/projects/pm-providers/registry.ts @@ -3,9 +3,9 @@ * * Providers register their wizard definition at module-load time by * calling `registerProviderWizard(def)` from the provider's frontend - * `index.ts`. The generic wizard renderer (`pm-wizard.tsx`) looks up the - * current provider here and falls back to the legacy per-provider - * branches when `getProviderWizard(id)` returns null. + * `index.ts`. The generic wizard renderer (`pm-wizard.tsx`) lists provider + * definitions from this registry for the picker and looks up the active + * provider here before rendering manifest-driven steps. */ import type { ProviderWizardDefinition } from './types.js'; diff --git a/web/src/components/projects/pm-providers/trello/auth.ts b/web/src/components/projects/pm-providers/trello/auth.ts new file mode 100644 index 000000000..302e29649 --- /dev/null +++ b/web/src/components/projects/pm-providers/trello/auth.ts @@ -0,0 +1,15 @@ +import type { ProviderAuthMetadata, ProviderCredentialPersistenceMapping } from '../types.js'; + +export const trelloAuthMetadata: ProviderAuthMetadata = { + rawCredentials: [ + { role: 'api_key', stateField: 'trelloApiKey' }, + { role: 'token', stateField: 'trelloToken' }, + ], + storedCredentials: { fallbackWhenStateFieldEmpty: 'trelloApiKey' }, + missingCredentialsMessage: 'Enter both credentials before verifying', +}; + +export const trelloCredentialPersistence: readonly ProviderCredentialPersistenceMapping[] = [ + { envVarKey: 'TRELLO_API_KEY', stateField: 'trelloApiKey', label: 'Trello API Key' }, + { envVarKey: 'TRELLO_TOKEN', stateField: 'trelloToken', label: 'Trello Token' }, +]; diff --git a/web/src/components/projects/pm-providers/trello/hooks.ts b/web/src/components/projects/pm-providers/trello/hooks.ts index 803bf333d..388e80ae1 100644 --- a/web/src/components/projects/pm-providers/trello/hooks.ts +++ b/web/src/components/projects/pm-providers/trello/hooks.ts @@ -4,6 +4,7 @@ import { useEffect } from 'react'; import { trpcClient } from '@/lib/trpc.js'; import { useProviderCustomFieldCreation, useProviderLabelCreation } from '../../pm-wizard-hooks.js'; import type { WizardAction, WizardState } from '../../pm-wizard-state.js'; +import type { ProviderAuthMetadata } from '../types.js'; // ============================================================================ // Trello Discovery @@ -112,13 +113,16 @@ export function useTrelloDiscovery( // ============================================================================ export function useTrelloLabelCreation( + providerId: string, + auth: ProviderAuthMetadata, state: WizardState, dispatch: Dispatch, projectId: string, ) { return useProviderLabelCreation( { - providerId: 'trello', + providerId, + auth, getContainerId: (s) => s.trelloBoardId, containerError: 'Board must be selected before creating a label', addLabel: (label) => ({ type: 'ADD_TRELLO_BOARD_LABEL', label }), @@ -135,13 +139,16 @@ export function useTrelloLabelCreation( // ============================================================================ export function useTrelloCustomFieldCreation( + providerId: string, + auth: ProviderAuthMetadata, state: WizardState, dispatch: Dispatch, projectId: string, ) { return useProviderCustomFieldCreation( { - providerId: 'trello', + providerId, + auth, getContainerId: (s) => s.trelloBoardId, containerError: 'Board must be selected before creating a custom field', addCustomField: (f) => ({ type: 'ADD_TRELLO_BOARD_CUSTOM_FIELD', customField: f }), diff --git a/web/src/components/projects/pm-providers/trello/state.ts b/web/src/components/projects/pm-providers/trello/state.ts new file mode 100644 index 000000000..6a31aa34e --- /dev/null +++ b/web/src/components/projects/pm-providers/trello/state.ts @@ -0,0 +1,139 @@ +export interface TrelloBoardOption { + id: string; + name: string; + url: string; +} + +export interface TrelloBoardDetails { + lists: Array<{ id: string; name: string }>; + labels: Array<{ id: string; name: string; color: string }>; + customFields: Array<{ id: string; name: string; type: string }>; +} + +export interface TrelloWizardStateSlice { + trelloApiKey: string; + trelloToken: string; + trelloBoardId: string; + trelloBoards: TrelloBoardOption[]; + trelloBoardDetails: TrelloBoardDetails | null; + trelloListMappings: Record; + trelloLabelMappings: Record; + trelloCostFieldId: string; +} + +interface VerificationState { + verificationResult: { provider: string; display: string } | null; + verifyError: string | null; +} + +export type TrelloWizardAction = + | { type: 'SET_TRELLO_API_KEY'; value: string } + | { type: 'SET_TRELLO_TOKEN'; value: string } + | { type: 'SET_TRELLO_BOARDS'; boards: TrelloBoardOption[] } + | { type: 'SET_TRELLO_BOARD_ID'; id: string } + | { type: 'SET_TRELLO_BOARD_DETAILS'; details: TrelloBoardDetails | null } + | { type: 'SET_TRELLO_LIST_MAPPING'; key: string; value: string } + | { type: 'SET_TRELLO_LABEL_MAPPING'; key: string; value: string } + | { type: 'SET_TRELLO_COST_FIELD'; id: string } + | { type: 'ADD_TRELLO_BOARD_LABEL'; label: { id: string; name: string; color: string } } + | { + type: 'ADD_TRELLO_BOARD_CUSTOM_FIELD'; + customField: { id: string; name: string; type: string }; + }; + +export function createInitialTrelloState(): TrelloWizardStateSlice { + return { + trelloApiKey: '', + trelloToken: '', + trelloBoardId: '', + trelloBoards: [], + trelloBoardDetails: null, + trelloListMappings: {}, + trelloLabelMappings: {}, + trelloCostFieldId: '', + }; +} + +export function isTrelloWizardAction(action: { type: string }): action is TrelloWizardAction { + return action.type.includes('TRELLO'); +} + +export function trelloWizardReducer( + state: T, + action: TrelloWizardAction, +): T { + switch (action.type) { + case 'SET_TRELLO_API_KEY': + return { + ...state, + trelloApiKey: action.value, + verificationResult: null, + verifyError: null, + }; + case 'SET_TRELLO_TOKEN': + return { + ...state, + trelloToken: action.value, + verificationResult: null, + verifyError: null, + }; + case 'SET_TRELLO_BOARDS': + return { ...state, trelloBoards: action.boards }; + case 'SET_TRELLO_BOARD_ID': + return { + ...state, + ...resetTrelloBoardState(action.id), + }; + case 'SET_TRELLO_BOARD_DETAILS': + return { ...state, trelloBoardDetails: action.details }; + case 'SET_TRELLO_LIST_MAPPING': + return { + ...state, + trelloListMappings: { ...state.trelloListMappings, [action.key]: action.value }, + }; + case 'SET_TRELLO_LABEL_MAPPING': + return { + ...state, + trelloLabelMappings: { ...state.trelloLabelMappings, [action.key]: action.value }, + }; + case 'SET_TRELLO_COST_FIELD': + return { ...state, trelloCostFieldId: action.id }; + case 'ADD_TRELLO_BOARD_LABEL': + if (!state.trelloBoardDetails) return state; + return { + ...state, + trelloBoardDetails: { + ...state.trelloBoardDetails, + labels: [...state.trelloBoardDetails.labels, action.label], + }, + }; + case 'ADD_TRELLO_BOARD_CUSTOM_FIELD': + if (!state.trelloBoardDetails) return state; + return { + ...state, + trelloBoardDetails: { + ...state.trelloBoardDetails, + customFields: [...state.trelloBoardDetails.customFields, action.customField], + }, + }; + } +} + +export function resetTrelloBoardState( + trelloBoardId: string, +): Pick< + TrelloWizardStateSlice, + | 'trelloBoardId' + | 'trelloBoardDetails' + | 'trelloListMappings' + | 'trelloLabelMappings' + | 'trelloCostFieldId' +> { + return { + trelloBoardId, + trelloBoardDetails: null, + trelloListMappings: {}, + trelloLabelMappings: {}, + trelloCostFieldId: '', + }; +} diff --git a/web/src/components/projects/pm-providers/trello/webhook-step.tsx b/web/src/components/projects/pm-providers/trello/webhook-step.tsx index 1d2953465..080ae13b8 100644 --- a/web/src/components/projects/pm-providers/trello/webhook-step.tsx +++ b/web/src/components/projects/pm-providers/trello/webhook-step.tsx @@ -20,12 +20,30 @@ import type { DataProps } from '@/lib/data-props.js'; import { WebhookUrlDisplayStep } from '../steps/webhook-url-display.js'; import type { ProviderWizardStepProps } from '../types.js'; -interface ActiveWebhook { +export interface ActiveWebhook { readonly id: string; readonly url: string; readonly active: boolean; } +export interface TrelloWebhookListData { + readonly trello?: ReadonlyArray<{ + readonly id: string | number; + readonly callbackURL: string; + readonly active: boolean; + }>; +} + +export function normalizeTrelloActiveWebhooks( + webhooksData: TrelloWebhookListData | undefined, +): ActiveWebhook[] { + return (webhooksData?.trello ?? []).map((webhook) => ({ + id: String(webhook.id), + url: webhook.callbackURL, + active: webhook.active, + })); +} + interface TrelloWebhookProviderHooks { readonly webhookUrl: string; readonly callbackBaseUrl: string; diff --git a/web/src/components/projects/pm-providers/trello/wizard.ts b/web/src/components/projects/pm-providers/trello/wizard.ts index 51919e52a..d7c4a0c3b 100644 --- a/web/src/components/projects/pm-providers/trello/wizard.ts +++ b/web/src/components/projects/pm-providers/trello/wizard.ts @@ -21,19 +21,20 @@ import type { ReactElement } from 'react'; import { useState } from 'react'; import { API_URL } from '@/lib/api.js'; import { trpc, trpcClient } from '@/lib/trpc.js'; -import { deriveActiveWebhooks } from '../../pm-wizard-state.js'; +import type { WizardState } from '../../pm-wizard-state.js'; import { ContainerPickStep } from '../steps/container-pick.js'; import { CustomFieldMappingStep } from '../steps/custom-field-mapping.js'; import { LabelMappingStep } from '../steps/label-mapping.js'; import { StatusMappingStep } from '../steps/status-mapping.js'; import type { ProviderWizardDefinition, ProviderWizardStepProps } from '../types.js'; +import { trelloAuthMetadata, trelloCredentialPersistence } from './auth.js'; import { useTrelloCustomFieldCreation, useTrelloDiscovery, useTrelloLabelCreation, } from './hooks.js'; import { TrelloOAuthStep } from './oauth-step.js'; -import { TrelloWebhookAdapter } from './webhook-step.js'; +import { normalizeTrelloActiveWebhooks, TrelloWebhookAdapter } from './webhook-step.js'; // CASCADE stage keys that map to Trello lists (one list per stage). export const TRELLO_LIST_SLOTS = [ @@ -247,18 +248,9 @@ function TrelloCustomFieldMappingAdapter({ export const trelloProviderWizard: ProviderWizardDefinition = { id: 'trello', label: 'Trello', - auth: { - rawCredentials: [ - { role: 'api_key', stateField: 'trelloApiKey' }, - { role: 'token', stateField: 'trelloToken' }, - ], - storedCredentials: { fallbackWhenStateFieldEmpty: 'trelloApiKey' }, - missingCredentialsMessage: 'Enter both credentials before verifying', - }, - credentialPersistence: [ - { envVarKey: 'TRELLO_API_KEY', stateField: 'trelloApiKey', label: 'Trello API Key' }, - { envVarKey: 'TRELLO_TOKEN', stateField: 'trelloToken', label: 'Trello Token' }, - ], + auth: trelloAuthMetadata, + formatVerificationDisplay: (me) => (me.displayName ? `@${me.displayName} (${me.name})` : me.name), + credentialPersistence: trelloCredentialPersistence, // Each step mirrors `trelloManifest.wizardSpec.steps` by id. steps: [ @@ -307,16 +299,42 @@ export const trelloProviderWizard: ProviderWizardDefinition = { ...(state.trelloCostFieldId ? { customFields: { cost: state.trelloCostFieldId } } : {}), }), + buildEditState: (initialConfig, configuredKeys) => { + const editState = { + provider: 'trello', + trelloBoardId: (initialConfig.boardId as string) ?? '', + trelloCostFieldId: + (initialConfig.customFields as Record | undefined)?.cost ?? '', + hasStoredCredentials: + configuredKeys.has('TRELLO_API_KEY') && configuredKeys.has('TRELLO_TOKEN'), + } satisfies Partial; + + const lists = initialConfig.lists as Record | undefined; + const labels = initialConfig.labels as Record | undefined; + + return { + ...editState, + ...(lists ? { trelloListMappings: lists } : {}), + ...(labels ? { trelloLabelMappings: labels } : {}), + }; + }, + isSetupComplete: (state) => { if (!state.trelloBoardId) return false; if (Object.keys(state.trelloListMappings).length === 0) return false; return isCredentialsComplete(state); }, - useProviderHooks: ({ state, dispatch, projectId, advanceToStep }) => { + useProviderHooks: ({ providerId, auth, state, dispatch, projectId, advanceToStep }) => { const discovery = useTrelloDiscovery(state, dispatch, advanceToStep, projectId ?? ''); - const labels = useTrelloLabelCreation(state, dispatch, projectId ?? ''); - const customField = useTrelloCustomFieldCreation(state, dispatch, projectId ?? ''); + const labels = useTrelloLabelCreation(providerId, auth, state, dispatch, projectId ?? ''); + const customField = useTrelloCustomFieldCreation( + providerId, + auth, + state, + dispatch, + projectId ?? '', + ); const queryClient = useQueryClient(); const [creatingSlot, setCreatingSlot] = useState(null); @@ -358,11 +376,7 @@ export const trelloProviderWizard: ProviderWizardDefinition = { (typeof window !== 'undefined' ? window.location.origin.replace(':5173', ':3000') : ''); const webhooksQuery = useQuery(trpc.webhooks.list.queryOptions({ projectId: projectId ?? '' })); - const activeTrelloWebhooks = deriveActiveWebhooks('trello', webhooksQuery.data) as Array<{ - id: string; - url: string; - active: boolean; - }>; + const activeTrelloWebhooks = normalizeTrelloActiveWebhooks(webhooksQuery.data); const createWebhookMutation = useMutation({ mutationFn: () => diff --git a/web/src/components/projects/pm-providers/types.ts b/web/src/components/projects/pm-providers/types.ts index 506625be3..9a5483da1 100644 --- a/web/src/components/projects/pm-providers/types.ts +++ b/web/src/components/projects/pm-providers/types.ts @@ -45,6 +45,8 @@ export interface ProviderWizardStepProps { * provider-specific discovery / label-creation hooks. */ export interface ProviderHooksContext { + readonly providerId: string; + readonly auth: ProviderAuthMetadata; readonly state: WizardState; readonly dispatch: React.Dispatch; readonly projectId: string | undefined; @@ -95,6 +97,12 @@ export interface ProviderWizardDefinition { readonly steps: readonly ProviderWizardStep[]; /** Provider-owned auth contract for raw credentials and stored fallback. */ readonly auth: ProviderAuthMetadata; + /** Formats the provider's normalized current-user discovery response. */ + readonly formatVerificationDisplay: (me: { + readonly id: string; + readonly name: string; + readonly displayName?: string; + }) => string; /** Normal provider credentials saved to project_credentials. */ readonly credentialPersistence: readonly ProviderCredentialPersistenceMapping[]; /** @@ -102,6 +110,15 @@ export interface ProviderWizardDefinition { * save API. Mirrors the existing `buildXxxIntegrationConfig` functions. */ readonly buildIntegrationConfig: (state: WizardState) => Record; + /** + * Hydrates provider-owned edit-mode wizard state from a saved integration + * config plus the project credential keys currently configured on the server. + * Raw credential values must not be returned. + */ + readonly buildEditState: ( + initialConfig: Record, + configuredKeys: ReadonlySet, + ) => Partial; /** True when all required steps report complete. */ readonly isSetupComplete: (state: WizardState) => boolean; /** diff --git a/web/src/components/projects/pm-wizard-hooks.ts b/web/src/components/projects/pm-wizard-hooks.ts index d73fd661c..27061fa2c 100644 --- a/web/src/components/projects/pm-wizard-hooks.ts +++ b/web/src/components/projects/pm-wizard-hooks.ts @@ -3,7 +3,7 @@ * Each hook encapsulates one concern to keep the main orchestrator thin. * * Generic hooks introduced in spec 013 refactor: - * - buildProviderAuthArg — single auth-arg builder for all three providers + * - buildProviderAuthArgFromMetadata — auth-arg builder driven by provider metadata * - useProviderLabelCreation— parameterized label-creation hook (replaces 2 copies) * - useProviderCustomFieldCreation — parameterized CF hook (replaces 2 copies) * - useSaveMutation — data-driven, no provider branching @@ -15,50 +15,21 @@ import { trpc, trpcClient } from '@/lib/trpc.js'; import { getCredentialRoles } from '../../../../src/config/integrationRoles.js'; import type { ProviderAuthMetadata, ProviderWizardDefinition } from './pm-providers/types.js'; import type { WizardAction, WizardState } from './pm-wizard-state.js'; -import { shouldUseStoredCredentials } from './pm-wizard-state.js'; - -// ============================================================================ -// Auth-arg builder — shared across all mutations -// ============================================================================ /** - * Build the `{ projectId }` or `{ credentials: ... }` portion of a tRPC - * request for any provider. Returns the stored-creds path when the user is - * editing an existing integration without re-typing their key. - * - * Extracted so every per-provider mutation stays below the cognitive- - * complexity threshold and a single place enforces the invariant. + * Returns true when the provider's credentials form fields are all non-empty. + * Driven entirely by the provider's `auth.rawCredentials` metadata so no + * shared file needs to change when a new provider declares its credential + * fields. Used to enable the "Verify Connection" button. */ -export function buildProviderAuthArg( +export function areCredentialsReadyFromMetadata( state: WizardState, - projectId: string, -): { projectId: string } | { credentials: Record } { - if (shouldUseStoredCredentials(state)) { - return { projectId }; - } - if (state.provider === 'trello') { - if (!state.trelloApiKey || !state.trelloToken) { - throw new Error('Enter both credentials before verifying'); - } - return { credentials: { api_key: state.trelloApiKey, token: state.trelloToken } }; - } - if (state.provider === 'linear') { - if (!state.linearApiKey) { - throw new Error('Enter your API key before verifying'); - } - return { credentials: { api_key: state.linearApiKey } }; - } - // jira - if (!state.jiraEmail || !state.jiraApiToken) { - throw new Error('Enter both credentials before verifying'); - } - return { - credentials: { - email: state.jiraEmail, - api_token: state.jiraApiToken, - base_url: state.jiraBaseUrl, - }, - }; + auth: ProviderAuthMetadata, +): boolean { + return auth.rawCredentials.every((cred) => { + const value = state[cred.stateField]; + return typeof value === 'string' && !!value; + }); } export function buildProviderAuthArgFromMetadata( @@ -66,8 +37,13 @@ export function buildProviderAuthArgFromMetadata( projectId: string, metadata: ProviderAuthMetadata, ): { projectId: string } | { credentials: Record } { + // In edit mode with stored credentials, pass { projectId } when the primary + // credential field is still empty (user hasn't re-typed their key). The + // fallbackWhenStateFieldEmpty field from the provider's auth metadata tells + // us which state field to check — no provider-specific branching needed. if ( - shouldUseStoredCredentials(state) && + state.isEditing && + state.hasStoredCredentials && !state[metadata.storedCredentials.fallbackWhenStateFieldEmpty] ) { return { projectId }; @@ -97,7 +73,7 @@ export function buildProviderAuthArgFromMetadata( */ export async function runPerLabelCreations(opts: { labelsToCreate: Array<{ slot: string; name: string; color?: string }>; - providerId: 'trello' | 'linear'; + providerId: string; containerId: string; authArg: { projectId: string } | { credentials: Record }; }): Promise<{ @@ -128,7 +104,9 @@ export async function runPerLabelCreations(opts: { // ============================================================================ interface LabelCreationConfig { - providerId: 'trello' | 'linear'; + providerId: string; + /** Provider-owned auth contract for raw credentials and stored fallback */ + auth: ProviderAuthMetadata; /** Returns the container ID (board / team) from state */ getContainerId: (state: WizardState) => string; /** Error when container not yet selected */ @@ -139,6 +117,23 @@ interface LabelCreationConfig { setLabelMapping: (slot: string, id: string) => WizardAction; } +export function buildProviderLabelCreationRequest( + config: Pick, + state: WizardState, + projectId: string, + vars: { name: string; color?: string }, +) { + const containerId = config.getContainerId(state); + if (!containerId) throw new Error(config.containerError); + return { + providerId: config.providerId, + containerId, + name: vars.name, + color: vars.color, + ...buildProviderAuthArgFromMetadata(state, projectId, config.auth), + }; +} + function useProviderLabelCreation( config: LabelCreationConfig, state: WizardState, @@ -147,16 +142,8 @@ function useProviderLabelCreation( ) { const createLabelMutation = useMutation({ mutationFn: (vars: { name: string; color?: string; slot: string }) => { - const containerId = config.getContainerId(state); - if (!containerId) throw new Error(config.containerError); - const authArg = buildProviderAuthArg(state, projectId); - return trpcClient.pm.discovery.createLabel.mutate({ - providerId: config.providerId, - containerId, - name: vars.name, - color: vars.color, - ...authArg, - }); + const request = buildProviderLabelCreationRequest(config, state, projectId, vars); + return trpcClient.pm.discovery.createLabel.mutate(request); }, onSuccess: (label, vars) => { dispatch(config.addLabel(label)); @@ -172,7 +159,7 @@ function useProviderLabelCreation( mutationFn: async (labelsToCreate: Array<{ slot: string; name: string; color?: string }>) => { const containerId = config.getContainerId(state); if (!containerId) throw new Error(config.containerError); - const authArg = buildProviderAuthArg(state, projectId); + const authArg = buildProviderAuthArgFromMetadata(state, projectId, config.auth); return runPerLabelCreations({ labelsToCreate, providerId: config.providerId, @@ -209,7 +196,9 @@ function useProviderLabelCreation( // ============================================================================ interface CustomFieldCreationConfig { - providerId: 'trello' | 'jira'; + providerId: string; + /** Provider-owned auth contract for raw credentials and stored fallback */ + auth: ProviderAuthMetadata; /** Returns the container ID from state (boardId / projectKey) */ getContainerId: (state: WizardState) => string; /** Error thrown when container not yet selected (required for Trello; omit for global providers like JIRA) */ @@ -222,6 +211,25 @@ interface CustomFieldCreationConfig { onError?: (error: unknown) => void; } +export function buildProviderCustomFieldCreationRequest( + config: Pick< + CustomFieldCreationConfig, + 'providerId' | 'auth' | 'getContainerId' | 'containerError' + >, + state: WizardState, + projectId: string, + vars: { name: string }, +) { + const containerId = config.getContainerId(state); + if (!containerId && config.containerError) throw new Error(config.containerError); + return { + providerId: config.providerId, + containerId: containerId || 'global', + name: vars.name, + ...buildProviderAuthArgFromMetadata(state, projectId, config.auth), + }; +} + function useProviderCustomFieldCreation( config: CustomFieldCreationConfig, state: WizardState, @@ -230,15 +238,8 @@ function useProviderCustomFieldCreation( ) { const createCustomFieldMutation = useMutation({ mutationFn: ({ name }: { name: string }) => { - const containerId = config.getContainerId(state); - if (!containerId && config.containerError) throw new Error(config.containerError); - const authArg = buildProviderAuthArg(state, projectId); - return trpcClient.pm.discovery.createCustomField.mutate({ - providerId: config.providerId, - containerId: containerId || 'global', - name, - ...authArg, - }); + const request = buildProviderCustomFieldCreationRequest(config, state, projectId, { name }); + return trpcClient.pm.discovery.createCustomField.mutate(request); }, onSuccess: (customField) => { dispatch( @@ -285,23 +286,6 @@ export function buildCurrentUserDiscoveryRequest( }; } -export function formatVerificationDisplay( - provider: string, - me: { id: string; name: string; displayName?: string }, -): string { - // Per-provider display formatting mirrors the pre-009/5 UX: - // Trello: "@{username} ({fullName})" — displayName is username - // JIRA: "{displayName} ({email})" — displayName is email - // Linear: "{displayName || name}" — displayName is the preferred handle - if (provider === 'trello') { - return me.displayName ? `@${me.displayName} (${me.name})` : me.name; - } - if (provider === 'jira') { - return me.displayName ? `${me.name} (${me.displayName})` : me.name; - } - return me.displayName || me.name; -} - export function useVerification( state: WizardState, dispatch: Dispatch, @@ -331,7 +315,7 @@ export function useVerification( if (provider !== state.provider) return; dispatch({ type: 'SET_VERIFICATION', - result: { provider, display: formatVerificationDisplay(provider, me) }, + result: { provider, display: manifestDef.formatVerificationDisplay(me) }, }); advanceToStep(3); }, diff --git a/web/src/components/projects/pm-wizard-state.ts b/web/src/components/projects/pm-wizard-state.ts index af21a6185..78a8c99a7 100644 --- a/web/src/components/projects/pm-wizard-state.ts +++ b/web/src/components/projects/pm-wizard-state.ts @@ -3,53 +3,64 @@ * Has zero imports from other pm-wizard files to avoid circular dependencies. */ import type { Reducer } from 'react'; +import { + createInitialJiraState, + INITIAL_JIRA_LABELS, + isJiraWizardAction, + type JiraProjectDetails, + type JiraProjectOption, + type JiraWizardAction, + jiraWizardReducer, +} from './pm-providers/jira/state.js'; +import { + createInitialLinearState, + INITIAL_LINEAR_LABELS, + isLinearWizardAction, + type LinearProjectOption, + type LinearTeamDetails, + type LinearTeamOption, + type LinearWizardAction, + linearWizardReducer, +} from './pm-providers/linear/state.js'; +import { + createInitialTrelloState, + isTrelloWizardAction, + type TrelloBoardDetails, + type TrelloBoardOption, + type TrelloWizardAction, + trelloWizardReducer, +} from './pm-providers/trello/state.js'; + +export type { + JiraProjectDetails, + JiraProjectOption, + LinearProjectOption, + LinearTeamDetails, + LinearTeamOption, + TrelloBoardDetails, + TrelloBoardOption, +}; +export { INITIAL_JIRA_LABELS, INITIAL_LINEAR_LABELS }; // ============================================================================ // Types // ============================================================================ -export interface TrelloBoardOption { - id: string; - name: string; - url: string; -} - -export interface TrelloBoardDetails { - lists: Array<{ id: string; name: string }>; - labels: Array<{ id: string; name: string; color: string }>; - customFields: Array<{ id: string; name: string; type: string }>; -} - -export interface JiraProjectOption { - key: string; - name: string; -} - -export interface JiraProjectDetails { - statuses: Array<{ name: string; id: string }>; - issueTypes: Array<{ name: string; subtask: boolean }>; - fields: Array<{ id: string; name: string; custom: boolean }>; -} - -export interface LinearTeamOption { - id: string; - name: string; - key: string; -} - -export interface LinearProjectOption { - id: string; - name: string; - icon: string | null; - color: string | null; -} - -export interface LinearTeamDetails { - states: Array<{ id: string; name: string; type: string }>; - labels: Array<{ id: string; name: string; color: string }>; -} - -export type Provider = 'trello' | 'jira' | 'linear'; +/** + * Provider identifier — an open string so new providers registered via the + * frontend barrel (`web/src/components/projects/pm-providers/index.ts`) can + * be dispatched without adding to a closed union here. + * + * Note: adding a new PM provider still requires updating `WizardState` with + * the provider's credential fields (e.g. `asanaApiKey: string`) and the + * reducer with the corresponding action types. Provider config-shape hydration + * belongs on that provider's `ProviderWizardDefinition.buildEditState`. + * The credential-readiness path + * (`areCredentialsReadyFromMetadata` in `pm-wizard-hooks.ts`) and the + * mutation auth path (`buildProviderAuthArgFromMetadata`) are metadata-driven + * and do NOT require changes. + */ +export type Provider = string; export interface WizardState { provider: Provider; @@ -100,97 +111,28 @@ export interface WizardState { export type WizardAction = | { type: 'SET_PROVIDER'; provider: Provider } - | { type: 'SET_TRELLO_API_KEY'; value: string } - | { type: 'SET_TRELLO_TOKEN'; value: string } - | { type: 'SET_JIRA_EMAIL'; value: string } - | { type: 'SET_JIRA_API_TOKEN'; value: string } - | { type: 'SET_JIRA_BASE_URL'; url: string } - | { type: 'SET_LINEAR_API_KEY'; value: string } | { type: 'SET_VERIFICATION'; result: { provider: Provider; display: string } | null; error?: string | null; } - | { type: 'SET_TRELLO_BOARDS'; boards: TrelloBoardOption[] } - | { type: 'SET_TRELLO_BOARD_ID'; id: string } - | { type: 'SET_JIRA_PROJECTS'; projects: JiraProjectOption[] } - | { type: 'SET_JIRA_PROJECT_KEY'; key: string } - | { type: 'SET_LINEAR_TEAMS'; teams: LinearTeamOption[] } - | { type: 'SET_LINEAR_TEAM_ID'; id: string } - | { type: 'SET_LINEAR_TEAM_DETAILS'; details: LinearTeamDetails | null } - | { type: 'SET_LINEAR_PROJECTS'; projects: LinearProjectOption[] } - | { type: 'SET_LINEAR_PROJECT_ID'; value: string } - | { type: 'SET_TRELLO_BOARD_DETAILS'; details: TrelloBoardDetails | null } - | { type: 'SET_JIRA_PROJECT_DETAILS'; details: JiraProjectDetails | null } - | { type: 'SET_TRELLO_LIST_MAPPING'; key: string; value: string } - | { type: 'SET_TRELLO_LABEL_MAPPING'; key: string; value: string } - | { type: 'SET_TRELLO_COST_FIELD'; id: string } - | { type: 'SET_JIRA_STATUS_MAPPING'; key: string; value: string } - | { type: 'SET_JIRA_ISSUE_TYPE'; key: string; value: string } - | { type: 'SET_JIRA_LABEL'; key: string; value: string } - | { type: 'SET_JIRA_COST_FIELD'; id: string } - | { type: 'SET_LINEAR_STATUS_MAPPING'; key: string; value: string } - | { type: 'SET_LINEAR_LABEL'; key: string; value: string } | { type: 'INIT_EDIT'; state: Partial } - | { type: 'ADD_TRELLO_BOARD_LABEL'; label: { id: string; name: string; color: string } } - | { type: 'ADD_LINEAR_TEAM_LABEL'; label: { id: string; name: string; color: string } } - | { - type: 'ADD_TRELLO_BOARD_CUSTOM_FIELD'; - customField: { id: string; name: string; type: string }; - } - | { type: 'ADD_JIRA_PROJECT_CUSTOM_FIELD'; field: { id: string; name: string; custom: boolean } }; + | TrelloWizardAction + | JiraWizardAction + | LinearWizardAction; // ============================================================================ // Initial state and constants // ============================================================================ -export const INITIAL_JIRA_LABELS: Record = { - processing: 'cascade-processing', - processed: 'cascade-processed', - error: 'cascade-error', - readyToProcess: 'cascade-ready', - auto: 'cascade-auto', -}; - -/** - * Linear label mappings store workflow-label **UUIDs**, not names, because - * Linear's GraphQL API rejects names for issueUpdate.labelIds. The wizard - * populates these from the team's existing labels or via the create-label - * button. Initial state is therefore empty — operators pick or create. - */ -export const INITIAL_LINEAR_LABELS: Record = {}; - export function createInitialState(): WizardState { return { provider: 'trello', - trelloApiKey: '', - trelloToken: '', - jiraEmail: '', - jiraApiToken: '', - jiraBaseUrl: '', - linearApiKey: '', verificationResult: null, verifyError: null, - trelloBoardId: '', - trelloBoards: [], - jiraProjectKey: '', - jiraProjects: [], - linearTeamId: '', - linearTeams: [], - linearProjectId: '', - linearProjects: [], - trelloBoardDetails: null, - jiraProjectDetails: null, - linearTeamDetails: null, - trelloListMappings: {}, - trelloLabelMappings: {}, - trelloCostFieldId: '', - jiraStatusMappings: {}, - jiraIssueTypes: {}, - jiraLabels: { ...INITIAL_JIRA_LABELS }, - jiraCostFieldId: '', - linearStatusMappings: {}, - linearLabels: { ...INITIAL_LINEAR_LABELS }, + ...createInitialTrelloState(), + ...createInitialJiraState(), + ...createInitialLinearState(), isEditing: false, hasStoredCredentials: false, }; @@ -211,246 +153,22 @@ export const wizardReducer: Reducer = (state, action) isEditing: state.isEditing, previousProvider: state.previousProvider, }; - case 'SET_TRELLO_API_KEY': - return { - ...state, - trelloApiKey: action.value, - verificationResult: null, - verifyError: null, - }; - case 'SET_TRELLO_TOKEN': - return { - ...state, - trelloToken: action.value, - verificationResult: null, - verifyError: null, - }; - case 'SET_JIRA_EMAIL': - return { - ...state, - jiraEmail: action.value, - verificationResult: null, - verifyError: null, - }; - case 'SET_JIRA_API_TOKEN': - return { - ...state, - jiraApiToken: action.value, - verificationResult: null, - verifyError: null, - }; - case 'SET_JIRA_BASE_URL': - return { ...state, jiraBaseUrl: action.url, verificationResult: null, verifyError: null }; - case 'SET_LINEAR_API_KEY': - return { - ...state, - linearApiKey: action.value, - verificationResult: null, - verifyError: null, - }; case 'SET_VERIFICATION': return { ...state, verificationResult: action.result, verifyError: action.error ?? null }; - case 'SET_TRELLO_BOARDS': - return { ...state, trelloBoards: action.boards }; - case 'SET_TRELLO_BOARD_ID': - return { - ...state, - trelloBoardId: action.id, - trelloBoardDetails: null, - trelloListMappings: {}, - trelloLabelMappings: {}, - trelloCostFieldId: '', - }; - case 'SET_JIRA_PROJECTS': - return { ...state, jiraProjects: action.projects }; - case 'SET_JIRA_PROJECT_KEY': - return { - ...state, - jiraProjectKey: action.key, - jiraProjectDetails: null, - jiraStatusMappings: {}, - jiraIssueTypes: {}, - jiraCostFieldId: '', - }; - case 'SET_LINEAR_TEAMS': - return { ...state, linearTeams: action.teams }; - case 'SET_LINEAR_TEAM_ID': - return { - ...state, - linearTeamId: action.id, - linearTeamDetails: null, - linearStatusMappings: {}, - // A new team invalidates the project list and any chosen project — - // Linear projects are team-scoped, so the previous selection is - // not guaranteed to belong to the new team. - linearProjectId: '', - linearProjects: [], - }; - case 'SET_LINEAR_PROJECTS': - return { ...state, linearProjects: action.projects }; - case 'SET_LINEAR_PROJECT_ID': - return { ...state, linearProjectId: action.value }; - case 'SET_LINEAR_TEAM_DETAILS': - return { ...state, linearTeamDetails: action.details }; - case 'SET_TRELLO_BOARD_DETAILS': - return { ...state, trelloBoardDetails: action.details }; - case 'SET_JIRA_PROJECT_DETAILS': - return { ...state, jiraProjectDetails: action.details }; - case 'SET_TRELLO_LIST_MAPPING': - return { - ...state, - trelloListMappings: { ...state.trelloListMappings, [action.key]: action.value }, - }; - case 'SET_TRELLO_LABEL_MAPPING': - return { - ...state, - trelloLabelMappings: { ...state.trelloLabelMappings, [action.key]: action.value }, - }; - case 'SET_TRELLO_COST_FIELD': - return { ...state, trelloCostFieldId: action.id }; - case 'SET_JIRA_STATUS_MAPPING': - return { - ...state, - jiraStatusMappings: { ...state.jiraStatusMappings, [action.key]: action.value }, - }; - case 'SET_JIRA_ISSUE_TYPE': - return { - ...state, - jiraIssueTypes: { ...state.jiraIssueTypes, [action.key]: action.value }, - }; - case 'SET_JIRA_LABEL': - return { - ...state, - jiraLabels: { ...state.jiraLabels, [action.key]: action.value }, - }; - case 'SET_JIRA_COST_FIELD': - return { ...state, jiraCostFieldId: action.id }; - case 'SET_LINEAR_STATUS_MAPPING': - return { - ...state, - linearStatusMappings: { ...state.linearStatusMappings, [action.key]: action.value }, - }; - case 'SET_LINEAR_LABEL': - return { - ...state, - linearLabels: { ...state.linearLabels, [action.key]: action.value }, - }; case 'INIT_EDIT': { const merged = { ...state, ...action.state, isEditing: true }; // Snapshot the loaded provider so a later SET_PROVIDER knows what to clean up. merged.previousProvider = merged.provider; return merged; } - case 'ADD_TRELLO_BOARD_LABEL': - if (!state.trelloBoardDetails) return state; - return { - ...state, - trelloBoardDetails: { - ...state.trelloBoardDetails, - labels: [...state.trelloBoardDetails.labels, action.label], - }, - }; - case 'ADD_LINEAR_TEAM_LABEL': - if (!state.linearTeamDetails) return state; - return { - ...state, - linearTeamDetails: { - ...state.linearTeamDetails, - labels: [...state.linearTeamDetails.labels, action.label], - }, - }; - case 'ADD_TRELLO_BOARD_CUSTOM_FIELD': - if (!state.trelloBoardDetails) return state; - return { - ...state, - trelloBoardDetails: { - ...state.trelloBoardDetails, - customFields: [...state.trelloBoardDetails.customFields, action.customField], - }, - }; - case 'ADD_JIRA_PROJECT_CUSTOM_FIELD': - if (!state.jiraProjectDetails) return state; - return { - ...state, - jiraProjectDetails: { - ...state.jiraProjectDetails, - fields: [...state.jiraProjectDetails.fields, action.field], - }, - }; default: + if (isTrelloWizardAction(action)) return trelloWizardReducer(state, action); + if (isJiraWizardAction(action)) return jiraWizardReducer(state, action); + if (isLinearWizardAction(action)) return linearWizardReducer(state, action); return state; } }; -// ============================================================================ -// Edit-mode state builder -// ============================================================================ - -/** - * Build a partial WizardState from an existing integration's config. - * Called when editing an existing PM integration. - * Note: Raw credential values are NOT pre-populated for security. When stored credentials - * exist in project_credentials, `hasStoredCredentials` is set true so the wizard can - * operate without re-entry. - */ -// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: restoring state from two provider config shapes -export function buildEditState( - provider: string, - initialConfig: Record, - configuredKeys: Set, -): Partial { - const editState: Partial = { - provider: provider as Provider, - }; - - if (provider === 'trello') { - editState.trelloBoardId = (initialConfig.boardId as string) ?? ''; - - const lists = initialConfig.lists as Record | undefined; - if (lists) editState.trelloListMappings = lists; - - const labels = initialConfig.labels as Record | undefined; - if (labels) editState.trelloLabelMappings = labels; - - const cf = initialConfig.customFields as Record | undefined; - editState.trelloCostFieldId = cf?.cost ?? ''; - - editState.hasStoredCredentials = - configuredKeys.has('TRELLO_API_KEY') && configuredKeys.has('TRELLO_TOKEN'); - } else if (provider === 'jira') { - editState.jiraBaseUrl = (initialConfig.baseUrl as string) ?? ''; - editState.jiraProjectKey = (initialConfig.projectKey as string) ?? ''; - - const statuses = initialConfig.statuses as Record | undefined; - if (statuses) editState.jiraStatusMappings = statuses; - - const issueTypes = initialConfig.issueTypes as Record | undefined; - if (issueTypes) editState.jiraIssueTypes = issueTypes; - - const labels = initialConfig.labels as Record | undefined; - if (labels) editState.jiraLabels = labels; - - const cf = initialConfig.customFields as Record | undefined; - editState.jiraCostFieldId = cf?.cost ?? ''; - - editState.hasStoredCredentials = - configuredKeys.has('JIRA_EMAIL') && configuredKeys.has('JIRA_API_TOKEN'); - } else if (provider === 'linear') { - editState.linearTeamId = (initialConfig.teamId as string) ?? ''; - editState.linearProjectId = (initialConfig.projectId as string) ?? ''; - - const statuses = initialConfig.statuses as Record | undefined; - if (statuses) editState.linearStatusMappings = statuses; - - const labels = initialConfig.labels as Record | undefined; - if (labels) editState.linearLabels = labels; - - editState.hasStoredCredentials = configuredKeys.has('LINEAR_API_KEY'); - } - - return editState; -} - // ============================================================================ // Step-completion helpers (pure functions) // ============================================================================ @@ -459,42 +177,12 @@ export function isStep1Complete(state: WizardState): boolean { return !!state.provider; } -export function isStep2Complete(state: WizardState): boolean { - if (state.isEditing && state.hasStoredCredentials) return true; - const credsReady = - state.provider === 'trello' - ? !!(state.trelloApiKey && state.trelloToken) - : state.provider === 'jira' - ? !!(state.jiraEmail && state.jiraApiToken && state.jiraBaseUrl) - : !!state.linearApiKey; - return credsReady && !!state.verificationResult; -} - -export function isStep3Complete(state: WizardState): boolean { - if (state.provider === 'trello') return !!state.trelloBoardId; - if (state.provider === 'jira') return !!state.jiraProjectKey; - return !!state.linearTeamId; -} - -export function isStep4Complete(state: WizardState): boolean { - if (state.provider === 'trello') return Object.keys(state.trelloListMappings).length > 0; - if (state.provider === 'jira') return Object.keys(state.jiraStatusMappings).length > 0; - return Object.keys(state.linearStatusMappings).length > 0; -} - -export function areCredentialsReady(state: WizardState): boolean { - if (state.provider === 'trello') return !!(state.trelloApiKey && state.trelloToken); - if (state.provider === 'jira') - return !!(state.jiraEmail && state.jiraApiToken && state.jiraBaseUrl); - return !!state.linearApiKey; -} - /** * Returns `true` when a wizard mutation (verify, createLabel, createCustomField) * should pass `projectId` to the backend — meaning: edit mode is active, the * provider has stored credentials in `project_credentials`, and the user has - * NOT re-typed the primary API key in the form (because `buildEditState` - * intentionally leaves raw credentials blank for security). + * NOT re-typed the primary API key in the form (because provider-owned edit + * hydration intentionally leaves raw credentials blank for security). * * `resolvePMCredentials` on the backend (`src/api/routers/pm-discovery.ts`) * resolves stored credentials when `projectId` is supplied, so this check @@ -509,76 +197,3 @@ export function shouldUseStoredCredentials(state: WizardState): boolean { if (state.provider === 'jira') return !state.jiraApiToken; return !state.linearApiKey; } - -/** - * Build the Trello integration config payload from wizard state. - * Pure function so it can be unit-tested without the React runtime. - */ -export function buildTrelloIntegrationConfig(state: WizardState): Record { - return { - boardId: state.trelloBoardId, - lists: state.trelloListMappings, - labels: state.trelloLabelMappings, - ...(state.trelloCostFieldId ? { customFields: { cost: state.trelloCostFieldId } } : {}), - }; -} - -/** - * Build the JIRA integration config payload from wizard state. - * Pure function so it can be unit-tested without the React runtime. - */ -export function buildJiraIntegrationConfig(state: WizardState): Record { - return { - projectKey: state.jiraProjectKey, - baseUrl: state.jiraBaseUrl, - statuses: state.jiraStatusMappings, - ...(Object.keys(state.jiraIssueTypes).length > 0 ? { issueTypes: state.jiraIssueTypes } : {}), - ...(Object.keys(state.jiraLabels).length > 0 ? { labels: state.jiraLabels } : {}), - ...(state.jiraCostFieldId ? { customFields: { cost: state.jiraCostFieldId } } : {}), - }; -} - -/** - * Build the Linear integration config payload from wizard state. - * Pure function so it can be unit-tested without the React runtime. - */ -export function buildLinearIntegrationConfig(state: WizardState): Record { - return { - teamId: state.linearTeamId, - ...(state.linearProjectId ? { projectId: state.linearProjectId } : {}), - statuses: state.linearStatusMappings, - ...(Object.keys(state.linearLabels).length > 0 ? { labels: state.linearLabels } : {}), - }; -} - -/** - * Map the provider's webhook listing into the shape expected by `WebhookStep`. - * Linear webhooks are configured manually outside the wizard; Trello/JIRA come - * from the corresponding API listing. - */ -export function deriveActiveWebhooks( - provider: Provider, - webhooksData: - | { - trello?: ReadonlyArray<{ id: string | number; callbackURL: string; active: boolean }>; - jira?: ReadonlyArray<{ id: string | number; url: string; enabled: boolean }>; - } - | undefined, -): Array<{ id: string; url: string; active: boolean }> { - if (provider === 'trello') { - return (webhooksData?.trello ?? []).map((w) => ({ - id: String(w.id), - url: w.callbackURL, - active: w.active, - })); - } - if (provider === 'jira') { - return (webhooksData?.jira ?? []).map((w) => ({ - id: String(w.id), - url: w.url, - active: w.enabled, - })); - } - // Linear: webhooks are configured manually - return []; -} diff --git a/web/src/components/projects/pm-wizard.tsx b/web/src/components/projects/pm-wizard.tsx index 453e5f73f..2c2c48dd6 100644 --- a/web/src/components/projects/pm-wizard.tsx +++ b/web/src/components/projects/pm-wizard.tsx @@ -3,25 +3,25 @@ import { CheckCircle, Globe, Loader2, XCircle } from 'lucide-react'; import { useEffect, useReducer, useRef, useState } from 'react'; import { Label } from '@/components/ui/label.js'; import { trpc } from '@/lib/trpc.js'; -// Side-effect imports register every PM provider's frontend wizard into -// the provider registry. With Linear migrated (006/4), every PM provider -// now renders via the manifest shell. -import './pm-providers/trello/index.js'; -import './pm-providers/jira/index.js'; -import './pm-providers/linear/index.js'; +// Single barrel import registers every PM provider's frontend wizard into the +// provider registry. New providers add one import to pm-providers/index.ts — +// this file never needs to change for a new provider. +import './pm-providers/index.js'; import { ManifestProviderWizardSection } from './pm-providers/manifest-section.js'; -import { getProviderWizard } from './pm-providers/registry.js'; +import { getProviderWizard, listProviderWizards } from './pm-providers/registry.js'; import type { ProviderWizardDefinition } from './pm-providers/types.js'; import { SaveStep } from './pm-wizard-common-steps.js'; -import { useSaveMutation, useVerification } from './pm-wizard-hooks.js'; +import { + areCredentialsReadyFromMetadata, + useSaveMutation, + useVerification, +} from './pm-wizard-hooks.js'; // Plan 011/5: the three legacy `pm-wizard-{trello,jira,linear}-steps.tsx` // files were deleted; all three providers now render exclusively through // the manifest path (see `./pm-providers//wizard.ts`). // Plan 012/4: `WebhookStep` + `LinearWebhookInfoPanel` + supporting hooks // deleted; every provider owns its webhook UX via the manifest path. import { - areCredentialsReady, - buildEditState, createInitialState, isStep1Complete, type WizardAction, @@ -38,18 +38,63 @@ import { WizardStep } from './wizard-shared.js'; // (manifestDef.steps[i].title). Only step 1 (provider picker) and the // legacy Webhook + Save slots have fixed titles; rendered inline. -const PROVIDER_LABELS: Record<'trello' | 'jira' | 'linear', string> = { - trello: 'Trello', - jira: 'JIRA', - linear: 'Linear', -}; +export function buildProviderSwitchConfirmationMessage(fromLabel: string, toLabel: string): string { + return `Switch PM provider from ${fromLabel} to ${toLabel}?\n\nYou'll need to re-enter credentials and re-map fields for ${toLabel}. The old provider's credentials will be deleted when you save.`; +} + +function confirmProviderSwitch(fromLabel: string, toLabel: string): boolean { + return window.confirm(buildProviderSwitchConfirmationMessage(fromLabel, toLabel)); +} + +export function buildEditStateFromProviderWizard( + provider: string, + initialConfig: Record, + configuredKeys: ReadonlySet, +): Partial { + const manifestDef = getProviderWizard(provider); + if (!manifestDef) { + throw new Error(`No PM provider wizard registered for ${provider}`); + } + return manifestDef.buildEditState(initialConfig, configuredKeys); +} -function confirmProviderSwitch( - from: 'trello' | 'jira' | 'linear', - to: 'trello' | 'jira' | 'linear', -): boolean { - return window.confirm( - `Switch PM provider from ${PROVIDER_LABELS[from]} to ${PROVIDER_LABELS[to]}?\n\nYou'll need to re-enter credentials and re-map fields for ${PROVIDER_LABELS[to]}. The old provider's credentials will be deleted when you save.`, +export function ProviderPicker({ + state, + dispatch, + advanceToStep, +}: { + readonly state: WizardState; + readonly dispatch: React.Dispatch; + readonly advanceToStep: (step: number) => void; +}) { + return ( +
+ +
+ {listProviderWizards().map((wizard) => ( + + ))} +
+
); } @@ -105,7 +150,14 @@ function ManifestStepsSection({ }: ManifestStepsSectionProps) { // Called exactly once — the whole point of this wrapper component. const providerHooks = - manifestDef.useProviderHooks?.({ state, dispatch, projectId, advanceToStep }) ?? {}; + manifestDef.useProviderHooks?.({ + providerId: manifestDef.id, + auth: manifestDef.auth, + state, + dispatch, + projectId, + advanceToStep, + }) ?? {}; return ( <> @@ -220,7 +272,11 @@ export function PMWizard({ if (initializedRef.current) return; initializedRef.current = true; const configuredKeys = new Set(credentialsQuery.data.map((c) => c.envVarKey)); - const editState = buildEditState(initialProvider, initialConfig, configuredKeys); + const editState = buildEditStateFromProviderWizard( + initialProvider, + initialConfig, + configuredKeys, + ); dispatch({ type: 'INIT_EDIT', state: editState }); // Plan 011/4: open all steps up to a comfortable ceiling; actual // step count is provider-dependent (Trello 7, JIRA 8, Linear 7). @@ -229,10 +285,9 @@ export function PMWizard({ // ---- Custom hooks ---- - // Is there a manifest-registered wizard for the active provider? If so, - // ManifestProviderWizardSection drives the rendering (and runs the - // provider's useProviderHooks internally). Unregistered providers fall - // through to the legacy per-provider branches. + // Every selectable provider must be registered in the frontend wizard + // registry. ManifestProviderWizardSection drives all provider-owned steps + // and runs the provider's useProviderHooks internally. const manifestDef = getProviderWizard(state.provider); if (!manifestDef) { throw new Error(`No PM provider wizard registered for ${state.provider}`); @@ -253,7 +308,9 @@ export function PMWizard({ // ---- Step status ---- - const credsReady = areCredentialsReady(state); + // Metadata-driven: reads rawCredentials from the provider's auth spec so + // no shared file needs to change when a new provider is added. + const credsReady = areCredentialsReadyFromMetadata(state, manifestDef.auth); function getStatus( stepNum: number, @@ -280,30 +337,7 @@ export function PMWizard({ isOpen={openSteps.has(1)} onToggle={() => toggleStep(1)} > -
- -
- {(['trello', 'jira', 'linear'] as const).map((p) => ( - - ))} -
-
+ {/* @@ -314,26 +348,24 @@ export function PMWizard({ * the final Save step. ManifestStepsSection calls useProviderHooks * exactly once regardless of step count (storm fix). */} - {manifestDef && ( - verifyMutation.mutate()} - verificationResult={state.verificationResult} - verifyError={state.verifyError} - hasStoredCredentials={state.hasStoredCredentials} - isEditing={state.isEditing} - /> - )} + verifyMutation.mutate()} + verificationResult={state.verificationResult} + verifyError={state.verifyError} + hasStoredCredentials={state.hasStoredCredentials} + isEditing={state.isEditing} + /> {/* Save slot. */}