feat: ask the requester whether an unknown project is lf and review lf names missing from pcc (lfx-cdp-insights-ops#815)

services/libs/data-access-layer/src/project-catalog/types.ts

@@ -64,7 +64,7 @@ export interface IRequestHints {
   segmentId?: string
 }
 
-export type RequestQuestionKind = 'did_you_mean' | 'lf_conflict' | 'pick_project'
+export type RequestQuestionKind = 'did_you_mean' | 'lf_conflict' | 'lf_confirm' | 'pick_project'
 
 export interface IRequestQuestionOption {
   id: string

services/libs/project-onboarding/src/botCopy.test.ts

@@ -29,6 +29,11 @@ describe('botCopy', () => {
       whatsNeeded: 'Connect GitHub to the project with this repo',
       action: 'connect_github',
     })
+    expect(describeReviewCase('not_in_pcc', 'Acme')).toEqual({
+      insightsState: 'Not in the foundation project list',
+      whatsNeeded: 'Check Acme with PCC, then create it in CDP, add this repo and mark as done',
+      action: 'manual',
+    })
     expect(describeReviewCase('legacy_connection', 'TiKV')).toEqual({
       insightsState: 'Uses the old GitHub connection',
       whatsNeeded: 'Add the repo by hand in CDP, then mark as done',

services/libs/project-onboarding/src/botCopy.ts

@@ -39,6 +39,7 @@ export type ReviewCase =
   | 'connect_github'
   | 'legacy_connection'
   | 'project_not_set_up'
+  | 'not_in_pcc'
   | 'owner_only_match'
   | 'handed_off'
   | 'unknown'
@@ -83,6 +84,12 @@ const REVIEW_CASE_COPY: Record<ReviewCase, IReviewCaseTemplate> = {
     whatsNeeded: (project) => `Create ${project} in CDP, add this repo, then mark as done`,
     action: 'manual',
   },
+  not_in_pcc: {
+    insightsState: 'Not in the foundation project list',
+    whatsNeeded: (project) =>
+      `Check ${project} with PCC, then create it in CDP, add this repo and mark as done`,
+    action: 'manual',
+  },
   owner_only_match: {
     insightsState: 'Only the GitHub owner matches LF projects',
     whatsNeeded: () =>

services/libs/project-onboarding/src/botThreadReply.test.ts

@@ -158,6 +158,18 @@ describe('botThreadReply', () => {
         reviewReason: "Acme is an LF project, but it isn't set up in LFX Insights yet",
       }),
     ).toContain("I've asked the CDP team to create it and add the repo; they'll let you know here.")
+    expect(
+      buildBotReviewReply({
+        ...project,
+        classificationKind: 'lf_not_in_pcc',
+        proposedAction: 'human_review',
+        matchedSegmentId: null,
+        reviewReason:
+          "Acme is described as an LF project, but it isn't in the foundation project list yet",
+      }),
+    ).toBe(
+      `👀 ${REPO_LABEL}: Acme is described as an LF project, but it isn't in the foundation project list yet. I've asked the CDP team to check it with PCC and add it; they'll let you know here.`,
+    )
     expect(
       buildBotReviewReply({
         ...project,

services/libs/project-onboarding/src/botThreadReply.ts

@@ -78,6 +78,8 @@ function reviewNextStep(project: BotReviewProject): string {
       return "I've asked the CDP team to add it by hand; they'll mark it done here once it's in."
     case 'project_not_set_up':
       return "I've asked the CDP team to create it and add the repo; they'll let you know here."
+    case 'not_in_pcc':
+      return "I've asked the CDP team to check it with PCC and add it; they'll let you know here."
     case 'owner_only_match':
     case 'handed_off':
     case 'unknown':

services/libs/project-onboarding/src/classifyCatalogRow.test.ts

@@ -156,10 +156,51 @@ describe('classifyCatalogRow', () => {
     expect(result.trace.parsed).toMatchObject({ declaredLf: true })
     expect(result.classification).toMatchObject({
       classificationKind: 'lf_not_in_pcc',
+      proposedAction: 'human_review',
+      action: 'review',
+      reviewReason:
+        "Acme is described as an LF project, but it isn't in the foundation project list yet",
+      requestQuestion: null,
+    })
+  })
+
+  it('asks a bot requester whether an unknown project is LF before evaluating it', async () => {
+    const bot = { provenance: 'slack-bot' as const, requestedBy: 'U1' }
+
+    const asked = await classifyCatalogRow(row(bot), deps())
+    expect(asked.classification).toMatchObject({
+      classificationKind: 'ambiguous',
       proposedAction: null,
-      action: 'skip',
+      action: 'ask',
+      requestQuestion: { kind: 'lf_confirm' },
+    })
+
+    const discovered = await classifyCatalogRow(row(), deps())
+    expect(discovered.classification).toMatchObject({
+      classificationKind: 'non_lf_new_project',
+      proposedAction: 'create_segment',
+      action: 'evaluate',
       requestQuestion: null,
     })
+
+    const confirmedLf = await classifyCatalogRow(
+      row({ ...bot, requestHints: { declaredLf: true } }),
+      deps(),
+    )
+    expect(confirmedLf.classification).toMatchObject({
+      classificationKind: 'lf_not_in_pcc',
+      proposedAction: 'human_review',
+      action: 'review',
+    })
+
+    const notLf = await classifyCatalogRow(
+      row({ ...bot, requestHints: { treatAsNew: true } }),
+      deps(),
+    )
+    expect(notLf.classification).toMatchObject({
+      classificationKind: 'non_lf_new_project',
+      action: 'evaluate',
+    })
   })
 
   it('stores the project name the LLM extracted from the request text', async () => {
@@ -343,10 +384,26 @@ describe('toCatalogClassification', () => {
     })
   })
 
+  it('asks for human review of an LF project missing from the foundation project list', () => {
+    expect(
+      toCatalogClassification({ kind: 'lf_not_in_pcc', projectName: 'Acme' }, { autoApply: true }),
+    ).toEqual({
+      classificationKind: 'lf_not_in_pcc',
+      proposedAction: 'human_review',
+      pccProjectId: null,
+      matchedProjectName: null,
+      matchedSegmentId: null,
+      action: 'review',
+      requestQuestion: null,
+      skipReason: null,
+      reviewReason:
+        "Acme is described as an LF project, but it isn't in the foundation project list yet",
+    })
+  })
+
   it.each<OnboardingResolution>([
     { kind: 'already_tracked', repoUrls: ['https://github.com/acme/one'] },
     { kind: 'non_github_source', nonGithubRepoUrls: ['https://gitlab.com/acme/one'] },
-    { kind: 'lf_not_in_pcc', projectName: 'Acme' },
     { kind: 'ambiguous', reason: 'x', candidates: [] },
   ])('proposes no action for $kind', (resolution) => {
     expect(toCatalogClassification(resolution, { autoApply: true })).toMatchObject({

services/libs/project-onboarding/src/classifyCatalogRow.ts

@@ -135,10 +135,16 @@ function toProposal(resolution: OnboardingResolution): IProposal {
         pccProjectId: null,
         ...NO_MATCH,
       }
+    case 'lf_not_in_pcc':
+      return {
+        classificationKind: resolution.kind,
+        proposedAction: 'human_review',
+        pccProjectId: null,
+        ...NO_MATCH,
+      }
     case 'already_tracked':
     case 'ambiguous':
     case 'non_github_source':
-    case 'lf_not_in_pcc':
       return {
         classificationKind: resolution.kind,
         proposedAction: null,
@@ -169,22 +175,21 @@ export async function classifyCatalogRow(
   deps: CatalogRowClassificationDeps,
 ): Promise<ICatalogRowClassification> {
   const trace = createClassificationTrace()
+  const canAsk = isSlackBotProvenance(row.provenance ?? null) && !!row.requestedBy
   const resolution = await resolveParsedRequest(
     parsedRequestSource(row, deps),
     deps.lookups,
     trace,
     row.requestHints ?? {},
+    { canAsk },
   )
 
   return {
     resolution,
     trace,
     classification: {
       projectName: resolvedProjectName(row, trace),
-      ...toCatalogClassification(resolution, {
-        autoApply: deps.autoApply,
-        canAsk: isSlackBotProvenance(row.provenance ?? null) && !!row.requestedBy,
-      }),
+      ...toCatalogClassification(resolution, { autoApply: deps.autoApply, canAsk }),
     },
   }
 }

services/libs/project-onboarding/src/classifyRequest.ts

@@ -15,6 +15,7 @@ import {
 } from './requestParser'
 import {
   IOnboardingRequestLookups,
+  IResolveOptions,
   OnboardingResolution,
   resolveOnboardingRequest,
 } from './requestResolver'
@@ -47,6 +48,7 @@ export async function resolveParsedRequest(
   lookups: IOnboardingRequestLookups,
   trace: IClassificationTrace,
   hints: IRequestHints = {},
+  options: IResolveOptions = {},
 ): Promise<OnboardingResolution> {
   const parsed = await parse()
   if (parsed.ok === false) {
@@ -55,7 +57,7 @@ export async function resolveParsedRequest(
   }
 
   trace.parsed = parsed.request
-  return resolveOnboardingRequest(parsed.request, traceLookups(lookups, trace), hints)
+  return resolveOnboardingRequest(parsed.request, traceLookups(lookups, trace), hints, options)
 }
 
 export async function resolveWithTrace(

services/libs/project-onboarding/src/requestQuestion.test.ts

@@ -45,6 +45,15 @@ const conflictQuestion = {
     { id: 'not_sure', label: 'Not sure, ask the CDP team', handOff: true },
   ],
 }
+const confirmQuestion = {
+  kind: 'lf_confirm' as const,
+  text: "I couldn't find Acme in the foundation project list. Is it a Linux Foundation project?",
+  options: [
+    { id: 'lf', label: "Yes, it's an LF project", hints: { declaredLf: true } },
+    { id: 'not_lf', label: "No, it's not LF", hints: { treatAsNew: true } },
+    { id: 'not_sure', label: 'Not sure, ask the CDP team', handOff: true },
+  ],
+}
 const row = {
   id: '8f1c2a44-5b7e-4d2f-9c1a-3e6f7a8b9c0d',
   repoUrl: 'https://github.com/acme/widget',
@@ -125,6 +134,9 @@ describe('answer values', () => {
     expect(handOffReason(named, conflictQuestion)).toBe(
       "the requester isn't sure whether Acme Widgets is the LF project Acme Labs",
     )
+    expect(handOffReason(named, confirmQuestion)).toBe(
+      "the requester isn't sure whether Acme Widgets is an LF project",
+    )
   })
 
   it('finds options and describes an answered question', () => {

services/libs/project-onboarding/src/requestQuestion.ts

@@ -108,6 +108,8 @@ export function handOffReason(
       return suggested
         ? `the requester isn't sure whether ${name ?? 'this repo'} is the LF project ${suggested}`
         : `the requester isn't sure whether ${name ?? 'this repo'} is an LF project`
+    case 'lf_confirm':
+      return `the requester isn't sure whether ${name ?? 'this project'} is an LF project`
   }
 }
 

services/libs/project-onboarding/src/requestResolver.test.ts

@@ -62,6 +62,51 @@ describe('resolveOnboardingRequest', () => {
     expect(result).toEqual({ kind: 'non_lf_new_project', projectName: 'Acme' })
   })
 
+  it('asks the requester whether an unknown project is LF when it can', async () => {
+    const result = await resolveOnboardingRequest(request(), lookups([]), {}, { canAsk: true })
+
+    expect(result).toEqual({
+      kind: 'needs_input',
+      candidates: [],
+      fallbackReason:
+        "Acme isn't in the foundation project list and the request doesn't say whether it is an LF project",
+      question: {
+        kind: 'lf_confirm',
+        text: "I couldn't find Acme in the foundation project list. Is it a Linux Foundation project?",
+        options: [
+          { id: 'lf', label: "Yes, it's an LF project", hints: { declaredLf: true } },
+          { id: 'not_lf', label: "No, it's not LF", hints: { treatAsNew: true } },
+          { id: 'not_sure', label: 'Not sure, ask the CDP team', handOff: true },
+        ],
+      },
+    })
+  })
+
+  it('does not ask about the LF status when the request already says it or has no name', async () => {
+    const ask = { canAsk: true }
+
+    expect(
+      await resolveOnboardingRequest(request({ declaredLf: false }), lookups([]), {}, ask),
+    ).toEqual({ kind: 'non_lf_new_project', projectName: 'Acme' })
+    expect(
+      await resolveOnboardingRequest(request({ declaredLf: true }), lookups([]), {}, ask),
+    ).toEqual({ kind: 'lf_not_in_pcc', projectName: 'Acme' })
+    expect(
+      await resolveOnboardingRequest(request({ projectName: null }), lookups([]), {}, ask),
+    ).toEqual({ kind: 'non_lf_new_project', projectName: null })
+  })
+
+  it('follows the LF answer on the next run', async () => {
+    const ask = { canAsk: true }
+
+    expect(
+      await resolveOnboardingRequest(request(), lookups([]), { declaredLf: true }, ask),
+    ).toEqual({ kind: 'lf_not_in_pcc', projectName: 'Acme' })
+    expect(
+      await resolveOnboardingRequest(request(), lookups([]), { treatAsNew: true }, ask),
+    ).toEqual({ kind: 'non_lf_new_project', projectName: 'Acme' })
+  })
+
   it('alerts when the request is declared LF but PCC has no match', async () => {
     const result = await resolveOnboardingRequest(request({ declaredLf: true }), lookups([]))
 

services/libs/project-onboarding/src/requestResolver.ts

@@ -45,6 +45,10 @@ export interface IOnboardingRequestLookups {
   findExclusivelyLfOwners?: (owners: string[]) => Promise<Set<string>>
 }
 
+export interface IResolveOptions {
+  canAsk?: boolean
+}
+
 export type CdpIntegrationAction = 'create_integration' | 'update_integration' | 'human_review'
 
 export type PccMatchLevel = 'exact' | 'strong' | 'weak' | 'none'
@@ -271,6 +275,21 @@ function lfConflict(
   )
 }
 
+function lfConfirm(projectName: string): OnboardingResolution {
+  return needsInput(
+    {
+      kind: 'lf_confirm',
+      text: `I couldn't find ${projectName} in the foundation project list. Is it a Linux Foundation project?`,
+      options: [
+        { id: 'lf', label: "Yes, it's an LF project", hints: { declaredLf: true } },
+        { id: 'not_lf', label: "No, it's not LF", hints: { treatAsNew: true } },
+        NOT_SURE_OPTION,
+      ],
+    },
+    `${projectName} isn't in the foundation project list and the request doesn't say whether it is an LF project`,
+  )
+}
+
 function repositoriesLabel(count: number | undefined): string {
   if (count === undefined) {
     return ''
@@ -426,6 +445,7 @@ async function resolveWithoutPccMatch(
   request: IParsedOnboardingRequest,
   lookups: IOnboardingRequestLookups,
   pccConfigured: boolean,
+  canAsk: boolean,
 ): Promise<OnboardingResolution> {
   const byOwner = await resolveByOwnerHeuristic(request, lookups)
   if (byOwner) {
@@ -444,13 +464,18 @@ async function resolveWithoutPccMatch(
       : ambiguous("I couldn't tell which project this repository belongs to")
   }
 
+  if (request.declaredLf === null && request.projectName && canAsk) {
+    return lfConfirm(request.projectName)
+  }
+
   return { kind: 'non_lf_new_project', projectName: request.projectName }
 }
 
 export async function resolveOnboardingRequest(
   original: IParsedOnboardingRequest,
   lookups: IOnboardingRequestLookups,
   hints: IRequestHints = {},
+  { canAsk = false }: IResolveOptions = {},
 ): Promise<OnboardingResolution> {
   const request = withHints(original, hints)
   if (hasNoRepositories(request)) {
@@ -488,7 +513,7 @@ export async function resolveOnboardingRequest(
   const { projectName } = request
   const pccConfigured = lookups.findPccCandidates !== undefined
   if (!projectName || !lookups.findPccCandidates) {
-    return resolveWithoutPccMatch(request, lookups, pccConfigured)
+    return resolveWithoutPccMatch(request, lookups, pccConfigured, canAsk)
   }
 
   const assessment = assessPccCandidates(projectName, await lookups.findPccCandidates(projectName))
@@ -501,5 +526,5 @@ export async function resolveOnboardingRequest(
     return didYouMean(projectName, assessment.best)
   }
 
-  return resolveWithoutPccMatch(request, lookups, pccConfigured)
+  return resolveWithoutPccMatch(request, lookups, pccConfigured, canAsk)
 }

services/libs/project-onboarding/src/reviewCard.test.ts

@@ -171,6 +171,14 @@ describe('resolveReviewCase', () => {
         matchedSegmentId: null,
       }),
     ).toBe('project_not_set_up')
+    expect(
+      resolveReviewCase({
+        ...row,
+        classificationKind: 'lf_not_in_pcc',
+        proposedAction: 'human_review',
+        matchedSegmentId: null,
+      }),
+    ).toBe('not_in_pcc')
     expect(
       resolveReviewCase({ ...row, proposedAction: 'human_review', matchedSegmentId: null }),
     ).toBe('owner_only_match')