Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

C7E-777: Implement support for required preset scopes in session policies

This PR implements support for required preset scopes in session policies, extending the functionality to message policies and ensuring the Skip button respects the isRequired attribute.

Changes

  • Updated SessionMessages type to include isRequired property
  • Modified CreateSessionProvider to preserve the isRequired property from presets for message policies
  • Implemented hasRequiredPolicy function in CreateSession.tsx to check for required policies in both contract methods and message policies
  • Updated Skip button to be disabled when required policies exist
  • Updated MessageCard UI to respect the isRequired property similar to ContractCard.tsx

Testing

  • Verified that the Skip button is disabled when required policies exist
  • Verified that message policies with isRequired=true cannot be toggled

Link to Devin run

https://app.devin.ai/sessions/391eeff9d425420f87a56d8d427a32bf

Requested by: Tarrence van As ([email protected])

@devin-ai-integration
Copy link
Contributor Author

Original prompt from Tarrence:

Hey Devin,

I need you to implement support for required preset scopes in session policies for the C7E-777 ticket. This feature will prevent users from toggling certain policies and skipping the session creation screen when required policies are present.

## Task Overview
We now support an `is_required` attribute in preset session policies in the controller-gg/presets repo, but it's not fully implemented across the system. Currently, it only works for contract methods but not for message policies, and the Skip button doesn't respect this attribute.

## Implementation Steps

### 1. Check `isRequired` property is supported on `SignMessagePolicy` type

### 2. Update Skip button logic in CreateSession.tsx
In `cartridge-gg/controller/packages/keychain/src/components/connect/CreateSession.tsx`, add logic to check for required policies and disable the Skip button when required policies exist:

```diff
+ // Function to check if any policy is required
+ const hasRequiredPolicy = useMemo(() => {
+   if (!policies) return false;
+   
+   // Check contract methods
+   if (policies.contracts) {
+     for (const address in policies.contracts) {
+       if (policies.contracts[address].methods.some(m => m.isRequired)) {
+         return true;
+       }
+     }
+   }
+   
+   // Check message policies
+   if (policies.messages) {
+     if (policies.messages.some(m => m.isRequired)) {
+       return true;
+     }
+   }
+   
+   return false;
+ }, [policies]);

// In the return section of CreateSessionLayout, update the Skip button:
<Button
  variant="secondary"
  onClick={onSkipSession}
- disabled={isConnecting}
+ disabled={isConnecting || hasRequiredPolicy}
  className="px-8"
>
  Skip
</Button>

3. Update CreateSessionProvider to handle required message policies

In cartridge-gg/controller/packages/keychain/src/context/session.tsx, modify the message policy initialization to handle the isRequired property:

// Set all message policyState to authorized
if (initialPolicies.messages) {
  initialPolicies.messages.forEach((message, i) => {
    message.id = `${i}-${message.domain.name}-${message.name}`;
    message.authorized = true;
+   // Set required flag if applicable (implementation depends on requirements)
+   // This is where additional logic would be needed to determine if a message
+   // policy should be required
  });
}

4. Update Message Card UI

Make sure that any message policy UI components respect the isRequired property similar to how it's implemented in ContractCard.tsx. Look for components that render message policies and ensure they disable toggles when isRequired is true.

Testing

Verify your changes by testing the following scenarios:

  • Create a session with required contract methods and verify they can't be toggled
  • Create a session with required message policies and verify they can't be toggled
  • Verify the Skip button is disabled when any required policy exists
  • Verify the Skip button works normally when no required policies exist

Submitting Your PR

When you're done, create a PR and include the ticket ID "C7E-777" in the PR description. Mention that this PR implements support for required preset scopes in session policies.

Let me know if you have any questions or need clarification on any part of this task.

@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@vercel
Copy link

vercel bot commented May 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
controller-example-next ✅ Ready (Inspect) Visit Preview May 15, 2025 4:13pm
keychain ✅ Ready (Inspect) Visit Preview May 15, 2025 4:13pm
keychain-storybook ✅ Ready (Inspect) Visit Preview May 15, 2025 4:13pm
profile ✅ Ready (Inspect) Visit Preview May 15, 2025 4:13pm

variant="secondary"
onClick={onSkipSession}
disabled={isConnecting}
disabled={isConnecting || hasRequiredPolicy}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should not render the button if is is required

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants