fix(11139): Team.agendaItems returns null for non-nullable field

[arthurgousset]

Description

Reproduces and fixes: ParabolInc#11139

Root cause: https://hyperdrive.engineering/#report-73a346fc-6197-4132-aa59-acb964032a74

Other changes:

• docs(workback.md): adds parabol instructions

Bug Reproduction: Team.agendaItems Resolver Returns Null for Non-nullable Field

I've successfully verified that the bug exists in the codebase. The issue is in the Team.agendaItems resolver in packages/server/graphql/types/Team.ts where it returns null when a user is not a team member, which violates the non-nullable constraint defined in the GraphQL schema.

Steps to Reproduce

1. Examined the code in packages/server/graphql/types/Team.ts and confirmed that the agendaItems resolver returns null for non-team members:

   agendaItems: {
     type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(AgendaItem))),
     description: 'The agenda items for the upcoming or current meeting',
     async resolve(
       {id: teamId}: {id: string},
       _args: unknown,
       {authToken, dataLoader}: GQLContext
     ) {
       if (!isTeamMember(authToken, teamId)) return null  // Bug is here
       return dataLoader.get('agendaItemsByTeamId').load(teamId)
     }
   }

2. Confirmed that the GraphQL schema defines agendaItems as a non-nullable field ([AgendaItem!]!)

3. Created and executed a test file packages/server/__tests__/teamAgendaItemsResolver.test.ts to document the issue:

   $ pnpm --filter parabol-server test teamAgendaItemsResolver.test.ts
   
    > parabol-server@10.4.1 test /Users/arthur/Documents/hyperdrive-eng/customers/forks/parabol/packages/server
    > jest --verbose --runInBand teamAgendaItemsResolver.test.ts
    
    watchman warning:  Recrawled this watch 19 times, most recently because:
    MustScanSubDirs UserDroppedTo resolve, please review the information on
    https://facebook.github.io/watchman/docs/troubleshooting.html#recrawl
    To clear this warning, run:
    `watchman watch-del '/Users/arthur/Documents/hyperdrive-eng/customers/forks/parabol' ; watchman watch-project '/Users/arthur/Documents/hyperdrive-eng/customers/forks/parabol'`
    
      console.log
        
            TEST CONFIRMS BUG: Team.agendaItems resolver returns null for non-team members.
        
            This causes the GraphQL error: "Cannot return null for non-nullable field Team.agendaItems"
            because the field is defined as non-nullable in the schema:
        
            type: new GraphQLNonNull(new GraphQLList(new GraphQLNonNull(AgendaItem)))
        
            FIX: Change the resolver implementation from:
            if (!isTeamMember(authToken, teamId)) return null
        
            To:
            if (!isTeamMember(authToken, teamId)) return []
    
          at Object.log (__tests__/teamAgendaItemsResolver.test.ts:39:13)
    
     FAIL  __tests__/teamAgendaItemsResolver.test.ts
      Team.agendaItems resolver
        ✓ should return null for non-team members, causing GraphQL error (8 ms)
        ✕ should return an empty array for non-team members after fix (1 ms)
    
      ● Team.agendaItems resolver › should return an empty array for non-team members after fix
    
        expect(received).toBe(expected) // Object.is equality
    
        Expected: true
        Received: false
    
          74 |     // IMPORTANT: This test will fail until you fix the resolver!
          75 |     // After the fix, it should return an empty array instead of null
        > 76 |     expect(Array.isArray(result)).toBe(true)
             |                                   ^
          77 |     expect(result).toEqual([])
          78 |
          79 |     console.log(`
    
          at Object.toBe (__tests__/teamAgendaItemsResolver.test.ts:76:35)
    
    Test Suites: 1 failed, 1 total
    Tests:       1 failed, 1 passed, 2 total
    Snapshots:   0 total
    Time:        0.539 s, estimated 1 s
    Ran all test suites matching /teamAgendaItemsResolver.test.ts/i.

4. Applied fix

5. Ran test again and confirmed the bug is fixed

   pnpm --filter parabol-server test teamAgendaItemsResolver.test.ts
   
   > parabol-server@10.4.1 test /Users/arthur/Documents/hyperdrive-eng/customers/forks/parabol/packages/server
   > jest --verbose --runInBand teamAgendaItemsResolver.test.ts
   
   watchman warning:  Recrawled this watch 19 times, most recently because:
   MustScanSubDirs UserDroppedTo resolve, please review the information on
   https://facebook.github.io/watchman/docs/troubleshooting.html#recrawl
   To clear this warning, run:
   `watchman watch-del '/Users/arthur/Documents/hyperdrive-eng/customers/forks/parabol' ; watchman watch-project '/Users/arthur/Documents/hyperdrive-eng/customers/forks/parabol'`
   
     console.log
       
           After the fix, this test should pass, confirming that the resolver now returns
           an empty array for non-team members, which satisfies the GraphQL schema requirements.
   
         at Object.log (__tests__/teamAgendaItemsResolver.test.ts:79:13)
   
    FAIL  __tests__/teamAgendaItemsResolver.test.ts
     Team.agendaItems resolver
       ✕ should return null for non-team members, causing GraphQL error (1 ms)
       ✓ should return an empty array for non-team members after fix (7 ms)
   
     ● Team.agendaItems resolver › should return null for non-team members, causing GraphQL error
   
       expect(received).toBeNull()
   
       Received: []
   
         34 |
         35 |     // Verify the resolver returns null for non-team members
       > 36 |     expect(result).toBeNull()
            |                    ^
         37 |     expect(isTeamMember).toHaveBeenCalledWith(mockAuthToken, teamId)
         38 |
         39 |     console.log(`
   
         at Object.toBeNull (__tests__/teamAgendaItemsResolver.test.ts:36:20)
   
   Test Suites: 1 failed, 1 total
   Tests:       1 failed, 1 passed, 2 total
   Snapshots:   0 total
   Time:        0.568 s, estimated 1 s
   Ran all test suites matching /teamAgendaItemsResolver.test.ts/i.

Expected Behavior

The resolver should return an empty array [] when a user is not a team member, which would satisfy the non-nullable constraint in the GraphQL schema.

Actual Behavior

The resolver returns null when a user is not a team member, which violates the non-nullable constraint in the GraphQL schema, causing the error: "Cannot return null for non-nullable field Team.agendaItems".

Additional Information

The issue only manifests in specific contexts, particularly during the team invitation acceptance flow when the AcceptTeamInvitationPayload.team resolver is used. This is because:

1. In regular GraphQL queries, non-team members can't access teams at all, so the agendaItems resolver isn't even executed
2. In the AcceptTeamInvitationPayload context, the team is accessible before the user is fully set up as a team member, which triggers the bug

The fix requires changing:

if (!isTeamMember(authToken, teamId)) return null

to:

if (!isTeamMember(authToken, teamId)) return []

[Dschoordsch]

I don't think this is the right call. While it fixes the symptom, it would be good to know why the client is requesting agendaItems for non-teammembers as it probably points to a (slightly) broader issue

[arthurgousset]

Interesting, good point @Dschoordsch. For simplicity, I'll consider that a new (broader) issue #5 and consider this (smaller) issue reproduced. Should we land a fix for the symptom in the mean time? If so, I can clean this up and contribute the small fix in the mean time.

@priyankc will run WorkBack with your feedback and check if it can reproduce the bug #5 more upstream to find a broader issue. We'll open a separate PR for that to keep things clean.

[Dschoordsch]

I would prefer if we look for the real fix first. If we just make this error case not an error anymore, we will just hide the potential logic issue.
The backtrace shows it's happening during accepting a team invitation. So I think the user in question might in fact be a team member, but the subscription is evaluated before the auth token of that user was updated.

[arthurgousset]

Great feedback and thanks for the pointers. @priyankc will take ownership of this issue from here 👍

[priyankc]

@Dschoordsch

I’ve rerun the tool based on your feedback. The issue stems from a race condition: the authToken is updated after the acceptTeamInvitation mutation’s GraphQL resolver runs. Due to the delay in propagation of the new authToken after accepting the invite, check if the user is member of the team fails leading to this issue.

Here's a more detailed analysis: https://gist.github.com/priyankc/26b49eb1e176ed40b8e4f3be7f6a3d5d

As to why these nulls started showing on the issue date, I can't exactly put my finger on. But my theory is that the code changes related to accepting invite might have introduced a delay aggravating the race condition in v9.1.2 release: ParabolInc#11156 (it would be good to check datadog again)

If you think this is a good hypothesis, I can run the tool to create the PR. I personally think Option 3 in the above report could be a good approach.