diff --git a/tests/unit/web/pm-wizard-state.test.ts b/tests/unit/web/pm-wizard-state.test.ts index f7feeb17..09d68a4a 100644 --- a/tests/unit/web/pm-wizard-state.test.ts +++ b/tests/unit/web/pm-wizard-state.test.ts @@ -14,6 +14,7 @@ import { isStep2Complete, isStep3Complete, isStep4Complete, + shouldUseStoredCredentials, wizardReducer, } from '../../../web/src/components/projects/pm-wizard-state.js'; @@ -619,6 +620,104 @@ describe('areCredentialsReady', () => { }); }); +describe('shouldUseStoredCredentials', () => { + // When editing an existing integration, the form does NOT pre-fill + // the API key for security — `hasStoredCredentials` is flipped true + // but e.g. `linearApiKey` is empty. Wizard mutations (verify, + // createLabel, createCustomField) detect this and pass `projectId` + // to the backend so it resolves the stored credentials. + // + // Fresh setup (not editing) → always use form-state credentials. + // Edit mode where the user re-typed a key → use the fresh key. + // Edit mode with stored creds + empty key → use projectId. + + it('linear: false in fresh-setup mode (no editing)', () => { + const state = { ...createInitialState(), provider: 'linear' as const }; + expect(shouldUseStoredCredentials(state)).toBe(false); + }); + + it('linear: true in edit mode with stored creds and empty apiKey', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'linear' as const, + isEditing: true, + hasStoredCredentials: true, + linearApiKey: '', + }; + expect(shouldUseStoredCredentials(state)).toBe(true); + }); + + it('linear: false in edit mode when user re-typed the apiKey', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'linear' as const, + isEditing: true, + hasStoredCredentials: true, + linearApiKey: 'lin_fresh_typed_key', + }; + expect(shouldUseStoredCredentials(state)).toBe(false); + }); + + it('trello: true in edit mode with stored creds and empty apiKey', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'trello' as const, + isEditing: true, + hasStoredCredentials: true, + trelloApiKey: '', + trelloToken: '', + }; + expect(shouldUseStoredCredentials(state)).toBe(true); + }); + + it('trello: false in edit mode when user re-typed the apiKey', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'trello' as const, + isEditing: true, + hasStoredCredentials: true, + trelloApiKey: 'fresh_key', + trelloToken: '', + }; + expect(shouldUseStoredCredentials(state)).toBe(false); + }); + + it('jira: true in edit mode with stored creds and empty apiToken', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'jira' as const, + isEditing: true, + hasStoredCredentials: true, + jiraEmail: '', + jiraApiToken: '', + }; + expect(shouldUseStoredCredentials(state)).toBe(true); + }); + + it('jira: false in edit mode when user re-typed the apiToken', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'jira' as const, + isEditing: true, + hasStoredCredentials: true, + jiraEmail: '', + jiraApiToken: 'fresh_token', + }; + expect(shouldUseStoredCredentials(state)).toBe(false); + }); + + it('false when edit mode but hasStoredCredentials is false (user deleted creds)', () => { + const state: WizardState = { + ...createInitialState(), + provider: 'linear' as const, + isEditing: true, + hasStoredCredentials: false, + linearApiKey: '', + }; + expect(shouldUseStoredCredentials(state)).toBe(false); + }); +}); + // ============================================================================ // buildEditState // ============================================================================ diff --git a/web/src/components/projects/pm-providers/jira/wizard.ts b/web/src/components/projects/pm-providers/jira/wizard.ts index 4db19ed8..62693db8 100644 --- a/web/src/components/projects/pm-providers/jira/wizard.ts +++ b/web/src/components/projects/pm-providers/jira/wizard.ts @@ -289,7 +289,7 @@ export const jiraProviderWizard: ProviderWizardDefinition = { useProviderHooks: ({ state, dispatch, projectId, advanceToStep }) => { const discovery = useJiraDiscovery(state, dispatch, advanceToStep, projectId ?? ''); - const customField = useJiraCustomFieldCreation(state, dispatch); + const customField = useJiraCustomFieldCreation(state, dispatch, projectId ?? ''); const queryClient = useQueryClient(); const onCreateCustomField = (_slotKey: string, name: string) => { diff --git a/web/src/components/projects/pm-providers/linear/wizard.ts b/web/src/components/projects/pm-providers/linear/wizard.ts index 552779a4..c8a80a90 100644 --- a/web/src/components/projects/pm-providers/linear/wizard.ts +++ b/web/src/components/projects/pm-providers/linear/wizard.ts @@ -251,7 +251,7 @@ export const linearProviderWizard: ProviderWizardDefinition = { useProviderHooks: ({ state, dispatch, projectId, advanceToStep }) => { const discovery = useLinearDiscovery(state, dispatch, advanceToStep, projectId ?? ''); - const labels = useLinearLabelCreation(state, dispatch); + const labels = useLinearLabelCreation(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/trello/wizard.ts b/web/src/components/projects/pm-providers/trello/wizard.ts index 49178fdb..e28c8f96 100644 --- a/web/src/components/projects/pm-providers/trello/wizard.ts +++ b/web/src/components/projects/pm-providers/trello/wizard.ts @@ -274,8 +274,8 @@ export const trelloProviderWizard: ProviderWizardDefinition = { useProviderHooks: ({ state, dispatch, projectId, advanceToStep }) => { const discovery = useTrelloDiscovery(state, dispatch, advanceToStep, projectId ?? ''); - const labels = useTrelloLabelCreation(state, dispatch); - const customField = useTrelloCustomFieldCreation(state, dispatch); + const labels = useTrelloLabelCreation(state, dispatch, projectId ?? ''); + const customField = useTrelloCustomFieldCreation(state, dispatch, projectId ?? ''); const queryClient = useQueryClient(); const [creatingSlot, setCreatingSlot] = useState(null); diff --git a/web/src/components/projects/pm-wizard-hooks.ts b/web/src/components/projects/pm-wizard-hooks.ts index 53ec0e5f..00a5c061 100644 --- a/web/src/components/projects/pm-wizard-hooks.ts +++ b/web/src/components/projects/pm-wizard-hooks.ts @@ -14,7 +14,7 @@ import type { WizardAction, WizardState, } from './pm-wizard-state.js'; -import { buildLinearIntegrationConfig } from './pm-wizard-state.js'; +import { buildLinearIntegrationConfig, shouldUseStoredCredentials } from './pm-wizard-state.js'; // ============================================================================ // Trello Discovery @@ -367,61 +367,68 @@ export function useLinearDiscovery( // Verification // ============================================================================ +/** + * Build the `{ projectId }` or `{ credentials: ... }` portion of a tRPC + * request, picking the stored-creds path when the user is editing an + * existing integration and hasn't re-typed the key. Extracted so the + * `verifyMutation` body stays below the cognitive-complexity threshold. + */ +function buildVerifyAuthArg( + 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 } }; + } + 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, + }, + }; +} + export function useVerification( state: WizardState, dispatch: React.Dispatch, advanceToStep: (step: number) => void, + projectId: string, ) { const verifyMutation = useMutation({ mutationFn: async () => { // Plan 010/2: restore the pre-009/5 "Verified as @username" UX. - // Calls the new `currentUser` discovery capability, which every - // provider implements by mapping its native `getMe()` response - // to `{ id, name, displayName? }`. A successful call simultaneously - // validates the credentials and gives us the identity string to - // display. + // Calls the `currentUser` discovery capability; every provider + // maps its native `getMe()` response to `{ id, name, displayName? }`. + // + // Edit-mode fallback: `buildVerifyAuthArg` returns `{ projectId }` + // when the user is editing with stored credentials but an empty + // API-key field, so the backend resolves the stored secret via + // `resolvePMCredentials` instead of requiring re-entry. const provider = state.provider; - if (provider === 'trello') { - if (!state.trelloApiKey || !state.trelloToken) { - throw new Error('Enter both credentials before verifying'); - } - const me = (await trpcClient.pm.discovery.discover.mutate({ - providerId: 'trello', - capability: 'currentUser', - args: {}, - credentials: { - api_key: state.trelloApiKey, - token: state.trelloToken, - }, - })) as { id: string; name: string; displayName?: string }; - return { provider: 'trello' as const, me }; - } - if (provider === 'linear') { - if (!state.linearApiKey) { - throw new Error('Enter your API key before verifying'); - } - const me = (await trpcClient.pm.discovery.discover.mutate({ - providerId: 'linear', - capability: 'currentUser', - args: {}, - credentials: { api_key: state.linearApiKey }, - })) as { id: string; name: string; displayName?: string }; - return { provider: 'linear' as const, me }; - } - if (!state.jiraEmail || !state.jiraApiToken) { - throw new Error('Enter both credentials before verifying'); - } + const authArg = buildVerifyAuthArg(state, projectId); const me = (await trpcClient.pm.discovery.discover.mutate({ - providerId: 'jira', + providerId: provider, capability: 'currentUser', args: {}, - credentials: { - email: state.jiraEmail, - api_token: state.jiraApiToken, - base_url: state.jiraBaseUrl, - }, + ...authArg, })) as { id: string; name: string; displayName?: string }; - return { provider: 'jira' as const, me }; + return { provider, me }; }, onSuccess: ({ provider, me }) => { // Ignore if provider changed while we were verifying @@ -461,23 +468,69 @@ export function useVerification( // (`webhooks.list/create/delete` + `callbackBaseUrl` formula) — // see `./pm-providers/{trello,jira,linear}/wizard.ts`. +/** + * Iterate `labelsToCreate` through `pm.discovery.createLabel`, collecting + * successes + per-name errors. Factored out so the two + * `createMissingLabelsMutation` bodies (Trello + Linear) stay below the + * biome cognitive-complexity threshold. + */ +async function runPerLabelCreations(opts: { + labelsToCreate: Array<{ slot: string; name: string; color?: string }>; + providerId: 'trello' | 'linear'; + containerId: string; + authArg: { projectId: string } | { credentials: Record }; +}): Promise<{ + successes: Array<{ id: string; name: string; color: string }>; + errors: Array<{ name: string; error: string }>; +}> { + const successes: Array<{ id: string; name: string; color: string }> = []; + const errors: Array<{ name: string; error: string }> = []; + for (const { name, color } of opts.labelsToCreate) { + try { + const label = await trpcClient.pm.discovery.createLabel.mutate({ + providerId: opts.providerId, + containerId: opts.containerId, + name, + color, + ...opts.authArg, + }); + successes.push(label); + } catch (err) { + errors.push({ name, error: err instanceof Error ? err.message : String(err) }); + } + } + return { successes, errors }; +} + // ============================================================================ // Trello Label Creation // ============================================================================ -export function useTrelloLabelCreation(state: WizardState, dispatch: React.Dispatch) { +export function useTrelloLabelCreation( + state: WizardState, + dispatch: React.Dispatch, + projectId: string, +) { const createLabelMutation = useMutation({ mutationFn: (vars: { name: string; color?: string; slot: string }) => { - if (!state.trelloApiKey || !state.trelloToken || !state.trelloBoardId) { - throw new Error('Missing credentials or board selection'); + if (!state.trelloBoardId) { + throw new Error('Board must be selected before creating a label'); + } + const useStored = shouldUseStoredCredentials(state); + if (!useStored && (!state.trelloApiKey || !state.trelloToken)) { + throw new Error('Missing credentials — enter them on the credentials step'); } // Plan 010/1: routes through generic pm.discovery.createLabel. + // Edit mode with stored creds → projectId path (see + // `shouldUseStoredCredentials` in pm-wizard-state.ts). return trpcClient.pm.discovery.createLabel.mutate({ providerId: 'trello', containerId: state.trelloBoardId, name: vars.name, color: vars.color, - credentials: { api_key: state.trelloApiKey, token: state.trelloToken }, + ...(useStored + ? { projectId } + : { credentials: { api_key: state.trelloApiKey, token: state.trelloToken } }), }); }, onSuccess: (label, vars) => { @@ -492,29 +545,22 @@ export function useTrelloLabelCreation(state: WizardState, dispatch: React.Dispa const createMissingLabelsMutation = useMutation({ mutationFn: async (labelsToCreate: Array<{ slot: string; name: string; color?: string }>) => { - if (!state.trelloApiKey || !state.trelloToken || !state.trelloBoardId) { - throw new Error('Missing credentials or board selection'); - } - // Plan 010/1: iterate single-item pm.discovery.createLabel client-side. - // Collect successes + errors into the same shape the old batch endpoint - // returned so onSuccess downstream logic doesn't need to change. - const successes: Array<{ id: string; name: string; color: string }> = []; - const errors: Array<{ name: string; error: string }> = []; - for (const { name, color } of labelsToCreate) { - try { - const label = await trpcClient.pm.discovery.createLabel.mutate({ - providerId: 'trello', - containerId: state.trelloBoardId, - name, - color, - credentials: { api_key: state.trelloApiKey, token: state.trelloToken }, - }); - successes.push(label); - } catch (err) { - errors.push({ name, error: err instanceof Error ? err.message : String(err) }); - } - } - return { successes, errors }; + if (!state.trelloBoardId) { + throw new Error('Board must be selected before creating labels'); + } + const useStored = shouldUseStoredCredentials(state); + if (!useStored && (!state.trelloApiKey || !state.trelloToken)) { + throw new Error('Missing credentials — enter them on the credentials step'); + } + const authArg = useStored + ? { projectId } + : { credentials: { api_key: state.trelloApiKey, token: state.trelloToken } }; + return runPerLabelCreations({ + labelsToCreate, + providerId: 'trello', + containerId: state.trelloBoardId, + authArg, + }); }, onSuccess: (result, labelsToCreate) => { // Handle successful label creations @@ -552,24 +598,33 @@ export function useTrelloLabelCreation(state: WizardState, dispatch: React.Dispa export function useTrelloCustomFieldCreation( state: WizardState, dispatch: React.Dispatch, + projectId: string, ) { const createCustomFieldMutation = useMutation({ // Plan 011/2: the shared custom-field-mapping step lets operators type // a name. `mutate({ name })` — callers without a preference pass // `{ name: 'Cost' }` to preserve the legacy default. mutationFn: ({ name }: { name: string }) => { - if (!state.trelloApiKey || !state.trelloToken || !state.trelloBoardId) { - throw new Error('Missing credentials or board selection'); + if (!state.trelloBoardId) { + throw new Error('Board must be selected before creating a custom field'); + } + const useStored = shouldUseStoredCredentials(state); + if (!useStored && (!state.trelloApiKey || !state.trelloToken)) { + throw new Error('Missing credentials — enter them on the credentials step'); } // Plan 010/1 (leftover caller): routes through pm.discovery.createCustomField. return trpcClient.pm.discovery.createCustomField.mutate({ providerId: 'trello', containerId: state.trelloBoardId, name, - credentials: { - api_key: state.trelloApiKey, - token: state.trelloToken, - }, + ...(useStored + ? { projectId } + : { + credentials: { + api_key: state.trelloApiKey, + token: state.trelloToken, + }, + }), }); }, onSuccess: (customField) => { @@ -599,12 +654,14 @@ export function useTrelloCustomFieldCreation( export function useJiraCustomFieldCreation( state: WizardState, dispatch: React.Dispatch, + projectId: string, ) { const createJiraCustomFieldMutation = useMutation({ // Plan 011/3: the shared custom-field-mapping step lets operators type // a name; callers without a preference pass `{ name: 'Cost' }`. mutationFn: ({ name }: { name: string }) => { - if (!state.jiraEmail || !state.jiraApiToken || !state.jiraBaseUrl) { + const useStored = shouldUseStoredCredentials(state); + if (!useStored && (!state.jiraEmail || !state.jiraApiToken || !state.jiraBaseUrl)) { throw new Error('Missing JIRA credentials or base URL'); } // Plan 010/1: routes through generic pm.discovery.createCustomField. @@ -614,11 +671,15 @@ export function useJiraCustomFieldCreation( providerId: 'jira', containerId: state.jiraProjectKey || 'global', name, - credentials: { - email: state.jiraEmail, - api_token: state.jiraApiToken, - base_url: state.jiraBaseUrl, - }, + ...(useStored + ? { projectId } + : { + credentials: { + email: state.jiraEmail, + api_token: state.jiraApiToken, + base_url: state.jiraBaseUrl, + }, + }), }); }, onSuccess: (field) => { @@ -771,11 +832,19 @@ export function useSaveMutation(projectId: string, state: WizardState) { // Linear Label Creation // ============================================================================ -export function useLinearLabelCreation(state: WizardState, dispatch: React.Dispatch) { +export function useLinearLabelCreation( + state: WizardState, + dispatch: React.Dispatch, + projectId: string, +) { const createLabelMutation = useMutation({ mutationFn: (vars: { name: string; color?: string; slot: string }) => { - if (!state.linearApiKey || !state.linearTeamId) { - throw new Error('Missing credentials or team selection'); + if (!state.linearTeamId) { + throw new Error('Team must be selected before creating a label'); + } + const useStored = shouldUseStoredCredentials(state); + if (!useStored && !state.linearApiKey) { + throw new Error('Missing credentials — enter them on the credentials step'); } // Plan 010/1: routes through generic pm.discovery.createLabel. return trpcClient.pm.discovery.createLabel.mutate({ @@ -783,7 +852,7 @@ export function useLinearLabelCreation(state: WizardState, dispatch: React.Dispa containerId: state.linearTeamId, name: vars.name, color: vars.color, - credentials: { api_key: state.linearApiKey }, + ...(useStored ? { projectId } : { credentials: { api_key: state.linearApiKey } }), }); }, onSuccess: (label, vars) => { @@ -798,27 +867,20 @@ export function useLinearLabelCreation(state: WizardState, dispatch: React.Dispa const createMissingLabelsMutation = useMutation({ mutationFn: async (labelsToCreate: Array<{ slot: string; name: string; color?: string }>) => { - if (!state.linearApiKey || !state.linearTeamId) { - throw new Error('Missing credentials or team selection'); - } - // Plan 010/1: iterate single-item pm.discovery.createLabel client-side. - const successes: Array<{ id: string; name: string; color: string }> = []; - const errors: Array<{ name: string; error: string }> = []; - for (const { name, color } of labelsToCreate) { - try { - const label = await trpcClient.pm.discovery.createLabel.mutate({ - providerId: 'linear', - containerId: state.linearTeamId, - name, - color, - credentials: { api_key: state.linearApiKey }, - }); - successes.push(label); - } catch (err) { - errors.push({ name, error: err instanceof Error ? err.message : String(err) }); - } + if (!state.linearTeamId) { + throw new Error('Team must be selected before creating labels'); } - return { successes, errors }; + const useStored = shouldUseStoredCredentials(state); + if (!useStored && !state.linearApiKey) { + throw new Error('Missing credentials — enter them on the credentials step'); + } + const authArg = useStored ? { projectId } : { credentials: { api_key: state.linearApiKey } }; + return runPerLabelCreations({ + labelsToCreate, + providerId: 'linear', + containerId: state.linearTeamId, + authArg, + }); }, onSuccess: (result, labelsToCreate) => { for (const label of result.successes) { diff --git a/web/src/components/projects/pm-wizard-state.ts b/web/src/components/projects/pm-wizard-state.ts index 99685951..0b8e931f 100644 --- a/web/src/components/projects/pm-wizard-state.ts +++ b/web/src/components/projects/pm-wizard-state.ts @@ -489,6 +489,27 @@ export function areCredentialsReady(state: WizardState): boolean { 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). + * + * `resolvePMCredentials` on the backend (`src/api/routers/pm-discovery.ts`) + * resolves stored credentials when `projectId` is supplied, so this check + * lets edit-mode mutations work without the user re-typing their key. + * + * Fresh setup (no `isEditing`) → false → mutation passes `credentials` from + * form state (current behavior). + */ +export function shouldUseStoredCredentials(state: WizardState): boolean { + if (!state.isEditing || !state.hasStoredCredentials) return false; + if (state.provider === 'trello') return !state.trelloApiKey; + if (state.provider === 'jira') return !state.jiraApiToken; + return !state.linearApiKey; +} + /** * Build the Linear integration config payload from wizard state. * Pure function so it can be unit-tested without the React runtime. diff --git a/web/src/components/projects/pm-wizard.tsx b/web/src/components/projects/pm-wizard.tsx index 12636903..78d4bbc1 100644 --- a/web/src/components/projects/pm-wizard.tsx +++ b/web/src/components/projects/pm-wizard.tsx @@ -116,7 +116,7 @@ export function PMWizard({ // through to the legacy per-provider branches. const manifestDef = getProviderWizard(state.provider); - const { verifyMutation } = useVerification(state, dispatch, advanceToStep); + const { verifyMutation } = useVerification(state, dispatch, advanceToStep, projectId); // Every PM provider (Trello 006/2, JIRA 006/3, Linear 006/4) composes its // discovery / label / custom-field / webhook hooks inside its own // useProviderHooks. The parent wizard no longer calls any provider- @@ -213,24 +213,30 @@ export function PMWizard({ stepIndex={entry.index} /> - {/* Verify Connection button still belongs on the first - manifest step (credentials). */} + {/* Verify Connection button belongs on the first manifest + step (credentials). Always render — edit mode with stored + credentials uses the `projectId` path on the backend, so + users can verify without re-typing the key. */} {isCredentials && (
- {(!state.isEditing || !state.hasStoredCredentials || credsReady) && ( - + + {!credsReady && state.isEditing && state.hasStoredCredentials && ( + Using stored credentials )} {state.verificationResult && (