-
Notifications
You must be signed in to change notification settings - Fork 914
feat: add organization scope for shared ports #18314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This reverts commit 408d70d.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restricted my review just to the proto changes.
@@ -101,4 +101,7 @@ Read [cursor rules](.cursorrules). | |||
|
|||
## Frontend | |||
|
|||
The frontend is contained in the site folder. | |||
|
|||
For building Frontend refer to [this document](docs/contributing/frontend.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this line and the next one directly contradict each other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seemed to help prevent claude from trying to run commands not mentioned in that doc from the root of the repo for me. 🤷♀️
@aslilac When I test port sharing using the Coder with Claude Code template, when I attempt to share a port on the organization level or change an existing shared port to organization, I get a 400 response with the error below. Even if this is expected, its a bit confusing because there is no indication in the UI of an error.
|
are you testing against a local backend or the dev backend? |
I am testing against dev. I do notice the error UI does eventually appear if I 1. click the port share button with organization selected 2. after the button press, click both the port input and the sharing level dropdown. So its just a bit convoluted to get the error message to appear. Ideally the error message would appear when the response is received after pressing the share port button. Maybe this issue was always there but its more obvious with organization port sharing? |
you're only hitting this error because the dev backend does not support the new type yet. I'm not really concerned about the error message being good because it should never happen. your frontend version doesn't match the backend version, which we don't usually optimize for. |
if you wanna test this locally, you'll need to set the |
Fair warning: I made Claude 4 Opus Thinking implement this for me, and then I spent a day working back from it's solution to end up here. Please forgive me sins if I missed something awful while reviewing the code. 🙃 It definitely needed a bunch of tweaking, and missed a bunch of tangential but necessary changes.
Closes #17247
When sharing a port, you can now choose "organization" as a level. This fills a middle ground between "literally any authenticated user" and "only me".