fix: report whether a runtime is still initializing in ListResources - #9861
fix: report whether a runtime is still initializing in ListResources#9861nishantmonu51 wants to merge 1 commit into
ListResources#9861Conversation
`ListResources` returns 200 with an empty list when security policies deny every resource, which is what a deny-by-default project does for users who are entitled to nothing. The frontend read that as "the runtime isn't ready yet" and polled forever behind a spinner that never resolved. Add an `initializing` flag to `ListResourcesResponse`, derived from controller state rather than the returned resources, so it stays meaningful when every resource is filtered out. `Controller.Initializing` latches: once the initial parse and reconcile completes it stays false, so a later model refresh does not make the instance look like it is building again. Also surface access-denied errors on the canvas and explore embed surfaces, which previously rendered nothing at all for a denied user. Claude-Session: https://claude.ai/code/session_016toijNcMp2pV3UQ2WoZUhc
nishantmonu51
added
Type:Bug
AdityaHegde
left a comment
AdityaHegde
left a comment
There was a problem hiding this comment.
Approving with some small nits.
@begelundmuller can you take a look at the reconcile code once?
| repeated Resource resources = 1; | ||
| string next_page_token = 2; | ||
| // True while the instance may still produce more resources, i.e. it has not finished its initial parse and reconcile. | ||
| // It is computed before security policies are applied, so it stays meaningful when every resource is denied. |
There was a problem hiding this comment.
This seems extremely specific comment for the issue this PR is fixing. May be just the first line at 856 is enough?
| return nil, err | ||
| } | ||
|
|
||
| // Clients can't infer this from the returned resources: |
There was a problem hiding this comment.
nit: since we have a comment on initializing field and on Initializing method, this is redundant.
| return ctx.Err() | ||
| } | ||
|
|
||
| // Initializing returns true until the controller has completed its initial parse and reconcile, |
There was a problem hiding this comment.
nit: this comment block should maybe add some info on how started and initialized are used instead of talking about callers.
| if (!resources || resources.length === 0) { | ||
| // No data yet (the query errored or hasn't resolved): keep polling so we | ||
| // pick up resources once the runtime responds. | ||
| if (!resources) { |
There was a problem hiding this comment.
How about moving the initializing check here? Feels like a good place to refetch at MAX_REFETCH_INTERVAL
It will refetch even if one relevant resource is available but IMO it will be good to have all relevant resources parsed before running the rest of the code.
2 participants
ListResourcesreturns 200 with an empty list when security policies deny every resource, which is what a deny-by-default project does for users entitled to nothing. Clients read that as "the runtime isn't ready yet" and polled forever behind a spinner that never resolved.initializingtoListResourcesResponse, computed fromController.Initializing()rather than from the returned resources, so it stays meaningful when the response is empty or narrowed bykind/path.Controller.Initializing()latches: once the initial parse and reconcile completes it stays false for the controller's lifetime, so a model refresh or a hiddenRefreshTriggerdoes not make the instance look like it is building again. Astartedflag set alongsideRun's initial enqueue closes the window where every resource is stillIDLEand the instance would report "done" before it had begun.useIsInitialBuildand the dashboards refetch interval now key off the flag instead of guessing from an empty list.ResolveCanvasnow retries network errors only, so a 403 surfaces immediately instead of after five retries.Checklist:
https://claude.ai/code/session_016toijNcMp2pV3UQ2WoZUhc