ref(project-state): Introduce a dedicated state for 'new allowed'#5998
ref(project-state): Introduce a dedicated state for 'new allowed'#5998Dav1dde wants to merge 4 commits into
Conversation
loewenheim
left a comment
loewenheim
left a comment
There was a problem hiding this comment.
LGTM. Am I understanding correctly that it would be nicer to send no project config, rather than one with all fields empty/defaulted, but this is left as future work?
| // | ||
| // To support this, the proxy Relay would have to act as a pure proxy and not fetch | ||
| // the project configuration from its own cache. | ||
| continue; |
There was a problem hiding this comment.
Would it make sense to log a warning here?
There was a problem hiding this comment.
I decided against it, because it would log warnings on the "correctly" setup proxy Relay, instead of the incorrectly setup managed Relay.
Meaning someone could generate these warnings from the outside maliciously and they wouldn't be actionable for the user running the proxy Relay.
But maybe I am also overthinking it.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b84c747. Configure here.
Dav1dde
force-pushed
the
dav1d/new-allowed
branch
from
b84c747 to
a488368
Compare
3 participants
Introduces a dedicated state in the
ProjectStatefor Proxy Relays which do not fetch project configs from upstream.This makes a few implicit decisions explicit and more obvious. This PR is largely without side-effects. The biggest "hidden" change is that
enabled()now returnsNonefor the new state, but all usages ofenabled()use it to check feature flags, which will always fail even with the previous dummy state.Also taking suggestions for a better name than
DummyAllowed.