From 719e83788031ef78daff523061cf63e8d3a9eb0f Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Thu, 13 Aug 2026 22:14:06 +0000 Subject: [PATCH 01/14] Make `is Any` annotation checks legal under `strict_equality` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mypy 1.18 rejects `if x is Any:` as `comparison-overlap` when `x` is declared as a union of concrete types. The comparison is correct at runtime: `get_args` on an annotation such as `dict[str, Any]` hands back the `typing.Any` object itself, so the converters meet it as a value. Only the declared parameter types disagree, since they do not mention `Any`. Route the comparison through `is_annotation_any`, whose parameter is typed `object` — the honest domain of what typing introspection returns. The converters' declared unions stay as they are, and the runtime behavior is identical: the same `is` comparison, one call deeper. Until now this error failed the build of every target whose closure reaches `reboot/api.py` under a fresh mypy run, which is how it was found: Bazel's remote cache had been serving stale mypy results, so CI never re-ran mypy over these files. Co-Authored-By: Claude Fable 5 --- reboot/api.py | 15 ++++++++++++++- reboot/pydantic_schema_to_zod.py | 3 ++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/reboot/api.py b/reboot/api.py index 8987d9f6..40c6d01e 100644 --- a/reboot/api.py +++ b/reboot/api.py @@ -42,6 +42,19 @@ typing.Type[Dict[str, Any]], ] + +def is_annotation_any(annotation: object) -> bool: + """Whether `annotation` is the `typing.Any` special form. + + `get_args` on an annotation such as `dict[str, Any]` hands back the + `typing.Any` object itself, so converters that recurse into + annotations meet it as a value. Comparing through an `object`-typed + parameter keeps the comparison legal under `strict_equality` for + callers whose declared parameter types do not mention `Any`. + """ + return annotation is Any + + # We don't allow passing arbitrary default values, only these empty # defaults, which also matches with Protobuf semantics. ALLOWED_DEFAULT_BY_FIELD_TYPE = { @@ -638,7 +651,7 @@ def _proto_to_pydantic( # 'None' directly. return None - if output_type is Any: + if is_annotation_any(output_type): # Reverse of the `Any` case in `_pydantic_to_proto`: a # `google.protobuf.Value` becomes the JSON value it holds. assert isinstance(input, Value) diff --git a/reboot/pydantic_schema_to_zod.py b/reboot/pydantic_schema_to_zod.py index 9b9c1fb8..6c41220e 100644 --- a/reboot/pydantic_schema_to_zod.py +++ b/reboot/pydantic_schema_to_zod.py @@ -16,6 +16,7 @@ Model, UserPydanticError, get_field_tag, + is_annotation_any, snake_to_camel, ) from reboot.fail import fail @@ -208,7 +209,7 @@ def pydantic_to_zod( # Currently only used for methods with no response. return 'z.void()' - if input is Any: + if is_annotation_any(input): # `Any` — e.g. a `dict[str, Any]` map value — accepts any JSON # value. `z.json()` is the schema our `zod-to-proto` converts # to a `google.protobuf.Value`. From ccccb71513633198c5380360d508f238b83f4529 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 11 Aug 2026 03:23:51 +0000 Subject: [PATCH 02/14] Allow all methods, not just MCP methods, to have a description A `description=` was only set on `McpMethodOptions.description`, so a Reader, Writer, Transaction or Workflow that was not also an MCP tool didn't have it. It is now set on `MethodOptions.description` for every method, and MCP tool and resource descriptions read from there. `McpMethodOptions.description` is deprecated but still read as a fallback, so protos that already set it keep their descriptions. Co-Authored-By: Claude Opus 5 (1M context) --- rbt/v1alpha1/options.proto | 8 +++- reboot/protoc_gen_reboot_generic.py | 18 ++++++-- reboot/pydantic_schema_to_proto.py | 17 ++++--- reboot/templates/reboot.py.j2 | 4 +- tests/reboot/protoc_gen_reboot_tests.py | 61 +++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 14 deletions(-) diff --git a/rbt/v1alpha1/options.proto b/rbt/v1alpha1/options.proto index 394b9cc8..9c8227ca 100644 --- a/rbt/v1alpha1/options.proto +++ b/rbt/v1alpha1/options.proto @@ -53,8 +53,9 @@ message McpMethodOptions { optional bool resource = 2; // Custom name for the tool/resource (defaults to method name). optional string name = 3; - // Description for the tool/resource. - optional string description = 4; + // Deprecated: write `MethodOptions.description` instead. Read as a + // fallback when it is the only description present. + optional string description = 4 [deprecated = true]; // Display title for the tool/resource. optional string title = 5; } @@ -102,6 +103,9 @@ message MethodOptions { // MCP options for exposing this method as a tool or resource. optional McpMethodOptions mcp = 9; + + // What this method does, in the author's own words. + optional string description = 10; } extend google.protobuf.MethodOptions { diff --git a/reboot/protoc_gen_reboot_generic.py b/reboot/protoc_gen_reboot_generic.py index e41efc71..1e3a4f3b 100644 --- a/reboot/protoc_gen_reboot_generic.py +++ b/reboot/protoc_gen_reboot_generic.py @@ -157,7 +157,6 @@ class ProtoMcpOptions: tool: bool resource: bool name: Optional[str] - description: Optional[str] title: Optional[str] @@ -167,6 +166,7 @@ class ProtoMethodOptions: constructor: bool state_streaming: bool has_errors: bool + description: Optional[str] mcp: Optional[ProtoMcpOptions] @@ -898,16 +898,28 @@ def _proto_method_options( tool=mcp.tool, resource=mcp.resource, name=mcp.name if mcp.HasField('name') else None, - description=mcp.description - if mcp.HasField('description') else None, title=mcp.title if mcp.HasField('title') else None, ) + description: Optional[str] = None + if method_options.HasField('description'): + description = method_options.description + elif ( + method_options.HasField('mcp') and + method_options.mcp.HasField('description') + ): + # An application that was created before + # `MethodOptions.description` will have the deprecated + # `mcp` description, which is permitted for backward + # compatibility. + description = method_options.mcp.description + return ProtoMethodOptions( kind=kind, constructor=self._is_method_constructor(method), state_streaming=state_streaming, has_errors=len(method_options.errors) > 0, + description=description, mcp=mcp_options, ) diff --git a/reboot/pydantic_schema_to_proto.py b/reboot/pydantic_schema_to_proto.py index f37ee679..2f774a1c 100644 --- a/reboot/pydantic_schema_to_proto.py +++ b/reboot/pydantic_schema_to_proto.py @@ -921,6 +921,16 @@ async def generate_proto_file_from_api( f" errors: [\"{type_name}{to_pascal_case(method_name)}Errors\"],\n" ) + # What the author said the method does, written + # whether or not the method is exposed to MCP. + if method_spec.description is not None: + # The description can contain `\` character, so we + # need to escape it for proto string literal. + await proto.write( + " description: " + f'"{_escape_string_for_proto(method_spec.description)}",\n' + ) + # MCP options for exposing method as tool/resource. if method_spec.mcp is not None: mcp = method_spec.mcp @@ -935,13 +945,6 @@ async def generate_proto_file_from_api( mcp_fields.append( f'name: "{_escape_string_for_proto(mcp.name)}"' ) - if method_spec.description is not None: - # The description can contain `\` character, so - # we need to escape it for proto string literal. - mcp_fields.append( - "description: " - f'"{_escape_string_for_proto(method_spec.description)}"' - ) if mcp.title is not None: # The title can contain `\` character, so we need # to escape it for proto string literal. diff --git a/reboot/templates/reboot.py.j2 b/reboot/templates/reboot.py.j2 index 2acaeb46..66db0879 100644 --- a/reboot/templates/reboot.py.j2 +++ b/reboot/templates/reboot.py.j2 @@ -2692,7 +2692,7 @@ class {{ state.proto.name }}BaseServicer(IMPORT_reboot_aio_servicers.Servicer): {% set tool_name = method.options.proto.mcp.name if method.options.proto.mcp.name else mcp_name_prefix + (method.proto.name | to_snake) %} {% set tool_title = method.options.proto.mcp.title if method.options.proto.mcp.title else method.proto.name %} {% set tool_description_suffix = "" if state.proto.auto_construct != AUTO_CONSTRUCT_UNSPECIFIED else " on " + state.proto.name %} -{% set tool_description = method.options.proto.mcp.description if method.options.proto.mcp.description else "Invoke " + method.proto.name + tool_description_suffix + "." %} +{% set tool_description = method.options.proto.description if method.options.proto.description else "Invoke " + method.proto.name + tool_description_suffix + "." %} {% set request_type = state.proto.name + "." + method.proto.name + "Request" %} # Tool for '{{ method.proto.full_name }}'. @@ -2741,7 +2741,7 @@ class {{ state.proto.name }}BaseServicer(IMPORT_reboot_aio_servicers.Servicer): {% set method_path = method.proto.name | to_snake %} {% set resource_name = method.options.proto.mcp.name if method.options.proto.mcp.name else mcp_name_prefix + method_path %} {% set resource_title = method.options.proto.mcp.title if method.options.proto.mcp.title else method.proto.name %} -{% set resource_description = method.options.proto.mcp.description if method.options.proto.mcp.description else state.proto.name + " state." %} +{% set resource_description = method.options.proto.description if method.options.proto.description else state.proto.name + " state." %} {% set request_type = state.proto.name + "." + method.proto.name + "Request" %} # Resource for '{{ method.proto.full_name }}'. diff --git a/tests/reboot/protoc_gen_reboot_tests.py b/tests/reboot/protoc_gen_reboot_tests.py index 964c4a79..c1a28ca5 100644 --- a/tests/reboot/protoc_gen_reboot_tests.py +++ b/tests/reboot/protoc_gen_reboot_tests.py @@ -4,6 +4,7 @@ from google.protobuf.compiler import plugin_pb2 from google.protobuf.descriptor_pb2 import FileDescriptorSet from google.protobuf.descriptor_pool import DescriptorPool +from rbt.v1alpha1 import options_pb2 from reboot.protoc_gen_reboot_generic import ( BaseFile, ProtocPlugin, @@ -173,6 +174,66 @@ def test_map_type(self) -> None: map_field_type = fields['metadata'] self.assertEqual(map_field_type, "dict[str, str]") + def _greet_method_options(self): + test_plugin(self.plugin, self.descriptor_set) + + template_data = self.plugin.proto_to_template_data[ + 'tests/reboot/greeter.proto'] + methods = { + method.proto.name: method + for method in template_data.clients[0].services[0].methods + } + return methods['Greet'].options.proto + + def _greet_method_descriptor(self): + file = self.descriptor_set.file[-1] + self.assertEqual(file.name, 'tests/reboot/greeter.proto') + + service = file.service[0] + self.assertEqual(service.name, 'GreeterMethods') + + methods = {method.name: method for method in service.method} + return methods['Greet'] + + def test_method_description_reaches_the_template(self) -> None: + method = self._greet_method_descriptor() + method.options.Extensions[options_pb2.method + ].description = 'Greet someone.' + + self.assertEqual( + self._greet_method_options().description, + 'Greet someone.', + ) + + def test_deprecated_mcp_description_still_reaches_the_template( + self + ) -> None: + # An application that was created before + # `MethodOptions.description` will have the deprecated `mcp` + # description, which is permitted for backward compatibility. + method = self._greet_method_descriptor() + method.options.Extensions[options_pb2.method + ].mcp.description = 'Greet someone.' + + self.assertEqual( + self._greet_method_options().description, + 'Greet someone.', + ) + + def test_method_description_wins_over_the_deprecated_one(self) -> None: + method = self._greet_method_descriptor() + options = method.options.Extensions[options_pb2.method] + options.description = 'What the author wrote.' + options.mcp.description = 'The deprecated spelling.' + + self.assertEqual( + self._greet_method_options().description, + 'What the author wrote.', + ) + + def test_method_without_a_description_has_none(self) -> None: + self.assertIsNone(self._greet_method_options().description) + class ToLowerCamelTest(unittest.TestCase): """Test the `to_lower_camel` Jinja filter override. From 88616079a4810764c7f233b870269f70de7594ee Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 11 Aug 2026 03:25:02 +0000 Subject: [PATCH 03/14] Add a dev-mode companion application that serves a dashboard The first of several changes, split so that each can be reviewed on its own and so the dashboard can be tried out while the rest is written. It is not a documented feature yet: nothing opens a page by itself, so seeing one means passing `--open-dashboard` or visiting the URL. `rbt dev run` now starts a second Reboot application alongside the developer's, with its own state store, holding what the dashboard needs. The companion watches the developer's `api/` directory and records what those files declare, so the dashboard can describe an application. It also serves the page itself. The page reads that schema reactively and renders one section per state type: its fields, and each method's kind, whether it constructs, whether it is reachable over MCP, its signature and the errors it raises. Dashboard state, such as which detail views are open and which are closed, is saved in Reboot state, so it survives a hot reload and an `rbt dev run` restart. Auto-open is complete but off. `_AUTO_OPEN_DASHBOARD` is False, so only `--open-dashboard` opens a page. We don't reopen the dashboard if the developer already has it open, and we use the `Presence` library to determine whether they do. Note that presence does not drain through a DevPod workstation's port forward, which is filed separately. This will eventually supersede the inspect dashboard at `/__/inspect`, which lists state instances and their values. It does not replace it yet and both exist meanwhile: this describes an application's API, its state types, their fields and their methods, and cannot yet show the data behind them. Co-Authored-By: Claude Opus 5 (1M context) --- rbt/dashboard/v1/BUILD.bazel | 67 ++ rbt/dashboard/v1/dashboard.proto | 173 +++++ rbt/dashboard/v1/package.json | 3 + rbt/std/presence/v1/presence.proto | 16 + rbt/v1alpha1/inspect/inspect.proto | 51 ++ reboot/BUILD.bazel | 1 + reboot/cli/commands/BUILD.bazel | 26 + reboot/cli/commands/dashboard.py | 245 +++++++ reboot/cli/commands/dev.py | 207 +++++- reboot/cli/common/BUILD.bazel | 1 + reboot/cli/common/cli.py | 12 +- reboot/dashboard/BUILD.bazel | 120 +++ reboot/dashboard/api_reader.py | 202 ++++++ reboot/dashboard/api_watcher.py | 190 +++++ reboot/dashboard/constants.py | 46 ++ reboot/dashboard/frontend/BUILD.bazel | 51 ++ reboot/dashboard/frontend/dashboard.css | 681 ++++++++++++++++++ reboot/dashboard/frontend/index.html | 18 + reboot/dashboard/frontend/src/constants.ts | 5 + reboot/dashboard/frontend/src/main.tsx | 527 ++++++++++++++ reboot/dashboard/frontend/tsconfig.json | 16 + reboot/dashboard/main.py | 84 +++ reboot/dashboard/servicers.py | 131 ++++ reboot/inspect/BUILD.bazel | 13 + reboot/inspect/describe_state_type.py | 141 ++++ reboot/std/presence/v1/presence.py | 9 + tests/reboot/cli/BUILD.bazel | 14 + tests/reboot/cli/dashboard_tests.py | 127 ++++ tests/reboot/cli/dev_tests.py | 16 + tests/reboot/dashboard/BUILD.bazel | 83 +++ tests/reboot/dashboard/api/shop/v1/helper.py | 3 + tests/reboot/dashboard/api/shop/v1/shop.py | 48 ++ tests/reboot/dashboard/api_reader_tests.py | 127 ++++ tests/reboot/dashboard/api_watcher_tests.py | 111 +++ tests/reboot/dashboard/application_tests.py | 102 +++ tests/reboot/dashboard/dashboard_tests.py | 418 +++++++++++ .../reboot/dashboard/open_dashboard_tests.py | 219 ++++++ tests/reboot/dashboard/preferences_tests.py | 166 +++++ 38 files changed, 4466 insertions(+), 4 deletions(-) create mode 100644 rbt/dashboard/v1/BUILD.bazel create mode 100644 rbt/dashboard/v1/dashboard.proto create mode 100644 rbt/dashboard/v1/package.json create mode 100644 reboot/cli/commands/dashboard.py create mode 100644 reboot/dashboard/BUILD.bazel create mode 100644 reboot/dashboard/api_reader.py create mode 100644 reboot/dashboard/api_watcher.py create mode 100644 reboot/dashboard/constants.py create mode 100644 reboot/dashboard/frontend/BUILD.bazel create mode 100644 reboot/dashboard/frontend/dashboard.css create mode 100644 reboot/dashboard/frontend/index.html create mode 100644 reboot/dashboard/frontend/src/constants.ts create mode 100644 reboot/dashboard/frontend/src/main.tsx create mode 100644 reboot/dashboard/frontend/tsconfig.json create mode 100644 reboot/dashboard/main.py create mode 100644 reboot/dashboard/servicers.py create mode 100644 reboot/inspect/describe_state_type.py create mode 100644 tests/reboot/cli/dashboard_tests.py create mode 100644 tests/reboot/dashboard/BUILD.bazel create mode 100644 tests/reboot/dashboard/api/shop/v1/helper.py create mode 100644 tests/reboot/dashboard/api/shop/v1/shop.py create mode 100644 tests/reboot/dashboard/api_reader_tests.py create mode 100644 tests/reboot/dashboard/api_watcher_tests.py create mode 100644 tests/reboot/dashboard/application_tests.py create mode 100644 tests/reboot/dashboard/dashboard_tests.py create mode 100644 tests/reboot/dashboard/open_dashboard_tests.py create mode 100644 tests/reboot/dashboard/preferences_tests.py diff --git a/rbt/dashboard/v1/BUILD.bazel b/rbt/dashboard/v1/BUILD.bazel new file mode 100644 index 00000000..84478778 --- /dev/null +++ b/rbt/dashboard/v1/BUILD.bazel @@ -0,0 +1,67 @@ +load( + "@com_github_reboot_dev_reboot//reboot:rules.bzl", + "js_proto_library", + "js_reboot_library", + "js_reboot_react_library", + "py_reboot_library", +) +load("@com_google_protobuf//bazel:proto_library.bzl", "proto_library") + +proto_library( + name = "dashboard_proto", + srcs = [ + ":dashboard.proto", + ], + visibility = ["//visibility:public"], + deps = [ + "@com_github_reboot_dev_reboot//rbt/v1alpha1:options_proto", + ], +) + +py_reboot_library( + name = "dashboard_py_reboot", + proto = "dashboard.proto", + proto_library = ":dashboard_proto", + visibility = ["//visibility:public"], +) + +js_proto_library( + name = "dashboard_js_proto", + package_json = ":package.json", + proto = "dashboard.proto", + proto_deps = [ + ":dashboard_proto", + # ISSUE(https://github.com/reboot-dev/mono/issues/3218): Until we can + # use `create_protoc_plugin_rule` we need to repeat the dependencies of + # the `proto_libraries` here. + "@com_github_reboot_dev_reboot//rbt/v1alpha1:options_proto", + "@com_google_protobuf//:descriptor_proto", + ], + visibility = ["//visibility:public"], +) + +js_reboot_library( + name = "dashboard_js_reboot", + srcs = [ + ":dashboard_proto", + ], + proto = "dashboard.proto", + visibility = ["//visibility:public"], + deps = [ + ":dashboard_js_proto", + ], +) + +js_reboot_react_library( + name = "dashboard_js_reboot_react", + srcs = [ + ":dashboard_js_proto", + ], + proto = "dashboard.proto", + proto_deps = [ + ":dashboard_proto", + "@com_github_reboot_dev_reboot//rbt/v1alpha1:options_proto", + "@com_google_protobuf//:descriptor_proto", + ], + visibility = ["//visibility:public"], +) diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto new file mode 100644 index 00000000..38368499 --- /dev/null +++ b/rbt/dashboard/v1/dashboard.proto @@ -0,0 +1,173 @@ +syntax = "proto3"; + +package rbt.dashboard.v1; + +import "rbt/v1alpha1/options.proto"; + +//////////////////////////////////////////////////////////////////////// + +// The information the developer dashboard needs to display data +// about the Reboot application being developed. + +message FieldInfo { + string name = 1; + string type = 2; +} + +message MethodInfo { + string name = 1; + string kind = 2; + repeated FieldInfo arguments = 3; + repeated FieldInfo returns = 4; + repeated string errors = 5; + optional string description = 6; + bool factory = 7; + bool mcp = 8; +} + +message StateTypeInfo { + string name = 1; + + // The file the developer declared it in, e.g. + // "bank/v1/account.py". + string file = 2; + repeated FieldInfo fields = 3; + repeated MethodInfo methods = 4; +} + +//////////////////////////////////////////////////////////////////////// + +// What the dashboard application has read of the developer's API +// files, so that a browser can read it without reaching the +// application itself. +message API { + option (rbt.v1alpha1.state) = { + }; + + // The state types the developer's API files declare, which exist + // before the application does. + repeated StateTypeInfo state_types = 1; + + // Why the API files could not be read, if they could not be. A + // half-written file is the normal case while someone is typing, and + // saying so beats showing nothing. + string error = 2; +} + +message APIGetRequest {} + +message APIGetResponse { + repeated StateTypeInfo state_types = 1; + string error = 2; +} + +message APIUpdateRequest { + repeated StateTypeInfo state_types = 1; + string error = 2; +} + +message APIUpdateResponse {} + +message APIWatchRequest {} + +message APIWatchResponse {} + +//////////////////////////////////////////////////////////////////////// + +// What the developer has told the dashboard about opening dashboards. +// +// Kept here rather than in a file under their project because it is a +// fact about their machine and their browser, not about their +// application, and nothing about it belongs in their repository. It +// survives a hot reload and an `rbt dev run` restart because the +// dashboard application's state store does. +message Preferences { + option (rbt.v1alpha1.state) = { + }; + + // Whether the developer has asked `rbt dev run` to stop opening a + // dashboard by itself. Recorded as the exception rather than as + // the rule, so that somebody who has never chosen gets a dashboard + // opened when nobody is looking at one. + // + // `--open-dashboard` opens one whatever this says. + bool suppress_open_on_restart = 1; + + // The state types whose methods the developer has opened on the + // dashboard, by fully qualified name such as `bank.v1.Account`. + // Held as the set that is open rather than the set that is closed, + // so that somebody who has clicked nothing gets a page where every + // state type shows its methods but none of their detail. + // + // Kept sorted, so that clicking two state types open in one order + // and then the other stores the same thing both times. + repeated string expanded_state_types = 2; +} + +message PreferencesGetRequest {} + +message PreferencesGetResponse { + bool suppress_open_on_restart = 1; + repeated string expanded_state_types = 2; +} + +message PreferencesSetSuppressOpenOnRestartRequest { + bool suppress_open_on_restart = 1; +} + +message PreferencesSetSuppressOpenOnRestartResponse {} + +message PreferencesSetExpandedRequest { + // The fully qualified name of one state type, spelled the way + // `StateTypeInfo` spells it. + string state_type = 1; + + bool expanded = 2; +} + +message PreferencesSetExpandedResponse {} + +//////////////////////////////////////////////////////////////////////// + +service APIMethods { + rpc Get(APIGetRequest) returns (APIGetResponse) { + option (rbt.v1alpha1.method).reader = { + }; + } + + rpc Update(APIUpdateRequest) returns (APIUpdateResponse) { + option (rbt.v1alpha1.method).writer = { + }; + } + + // Watches the developer's API files for as long as the dashboard + // application runs, reading each one that changes. + rpc Watch(APIWatchRequest) returns (APIWatchResponse) { + option (rbt.v1alpha1.method).workflow = { + }; + } +} + +//////////////////////////////////////////////////////////////////////// + +service PreferencesMethods { + rpc Get(PreferencesGetRequest) returns (PreferencesGetResponse) { + option (rbt.v1alpha1.method).reader = { + }; + } + + rpc SetSuppressOpenOnRestart(PreferencesSetSuppressOpenOnRestartRequest) + returns (PreferencesSetSuppressOpenOnRestartResponse) { + option (rbt.v1alpha1.method).writer = { + }; + } + + // One state type per call rather than the whole set, so that two + // tabs opening two different state types at the same moment do not + // each write back a set that predates the other's click. + rpc SetExpanded(PreferencesSetExpandedRequest) + returns (PreferencesSetExpandedResponse) { + option (rbt.v1alpha1.method).writer = { + }; + } +} diff --git a/rbt/dashboard/v1/package.json b/rbt/dashboard/v1/package.json new file mode 100644 index 00000000..3dbc1ca5 --- /dev/null +++ b/rbt/dashboard/v1/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/rbt/std/presence/v1/presence.proto b/rbt/std/presence/v1/presence.proto index 38ac3238..23e0528b 100644 --- a/rbt/std/presence/v1/presence.proto +++ b/rbt/std/presence/v1/presence.proto @@ -24,6 +24,15 @@ message Presence { * `Subscriber` using the API described there. */ service PresenceMethods { + /** + * Constructs the `Presence` instance, empty, so that `List` + * answers before the first `Subscribe`. + */ + rpc Create(CreateRequest) returns (CreateResponse) { + option (rbt.v1alpha1.method).writer = { + }; + } + /** * Registers a subscriber as present. * @@ -54,6 +63,13 @@ service PresenceMethods { //////////////////////////////////////////////////////////////////////// +// See `Create`. +message CreateRequest {} + +message CreateResponse {} + +//////////////////////////////////////////////////////////////////////// + // See `Subscribe`. message SubscribeRequest { // The subscriber to register as present. diff --git a/rbt/v1alpha1/inspect/inspect.proto b/rbt/v1alpha1/inspect/inspect.proto index 5031d26c..478a0e92 100644 --- a/rbt/v1alpha1/inspect/inspect.proto +++ b/rbt/v1alpha1/inspect/inspect.proto @@ -66,6 +66,57 @@ message GetStateResponse { //////////////////////////////////////////////////////////////////////// +// A field of a state, or an argument of a method. +message FieldInfo { + string name = 1; + + // Rendered for a reader, e.g. "str", "float", "AccountState". + string type = 2; +} + +message MethodInfo { + string name = 1; + + // One of "reader", "writer", "transaction" or "workflow". + string kind = 2; + + // The request message's fields, flattened. A method taking no + // request has none. + repeated FieldInfo arguments = 3; + + // The response's fields; empty when the method returns + // nothing. + repeated FieldInfo returns = 4; + + // Names of the error types the method declares it may raise. + repeated string errors = 5; + + // The description its author wrote, when there is one. + optional string description = 6; + + // Whether this method is a factory, constructing the state + // rather than requiring it to already exist. + bool factory = 7; + + // Whether the method is exposed as an MCP tool or resource. + bool mcp = 8; +} + +message StateTypeInfo { + // Fully qualified, e.g. "bank.v1.Account". + string name = 1; + + // The file the developer declared it in, e.g. + // "bank/v1/account.py". + string file = 2; + + repeated FieldInfo fields = 3; + + repeated MethodInfo methods = 4; +} + +//////////////////////////////////////////////////////////////////////// + service Inspect { // The list of state types in an application is static, however, we // make this a streaming RPC so that the client can hear when it diff --git a/reboot/BUILD.bazel b/reboot/BUILD.bazel index 2bb7b4cd..c7d6d83d 100644 --- a/reboot/BUILD.bazel +++ b/reboot/BUILD.bazel @@ -516,6 +516,7 @@ py_library( ":python_thirdparty", "//reboot/aio:python", "//reboot/cli:main_py", + "//reboot/dashboard:main_py", "//reboot/mcp:python", "//reboot/nodejs:python", "@com_github_reboot_dev_reboot//protoc_gen_mypy_plugin:protoc-gen-mypy", diff --git a/reboot/cli/commands/BUILD.bazel b/reboot/cli/commands/BUILD.bazel index 6f059898..763242c8 100644 --- a/reboot/cli/commands/BUILD.bazel +++ b/reboot/cli/commands/BUILD.bazel @@ -1,6 +1,27 @@ load("@rbt_pypi//:requirements.bzl", "requirement") load("@rules_python//python:defs.bzl", "py_library") +py_library( + name = "dashboard_py", + srcs = ["dashboard.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":dev_py", + "//reboot:version_py", + "//reboot/cli/common:directories_py", + "//reboot/cli/common:rc_py", + "//reboot/cli/common:subprocesses_py", + "//reboot/cli/common:terminal_py", + "//reboot/dashboard:constants_py", + # The application this command spawns as a subprocess, so + # that it is importable from the CLI's own interpreter. + "//reboot/dashboard:main_py", + "@com_github_reboot_dev_reboot//reboot:settings_py", + "@com_github_reboot_dev_reboot//reboot/aio/backoff:python", + ], +) + py_library( name = "dev_py", srcs = ["dev.py"], @@ -12,6 +33,10 @@ py_library( requirement("cryptography"), requirement("python-dotenv"), ":generate_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + "//rbt/std/presence/v1:presence_py_reboot", + "//reboot/aio:aborted_py", + "//reboot/aio:external_py", "//reboot/cli/common:directories_py", "//reboot/cli/common:frontend_py", "//reboot/cli/common:monkeys_py", @@ -20,6 +45,7 @@ py_library( "//reboot/cli/common:transpile_py", "//reboot/cli/common:watch_py", "//reboot/controller:plan_makers_py", + "//reboot/dashboard:constants_py", "//reboot/server:local_envoy_factory_py", "@com_github_reboot_dev_reboot//reboot:settings_py", "@com_github_reboot_dev_reboot//reboot/aio:exceptions_py", diff --git a/reboot/cli/commands/dashboard.py b/reboot/cli/commands/dashboard.py new file mode 100644 index 00000000..35f51ea0 --- /dev/null +++ b/reboot/cli/commands/dashboard.py @@ -0,0 +1,245 @@ +"""The `rbt dashboard` command, which runs the developer dashboard.""" +import argparse +import asyncio +import os +import secrets +import shutil +import sys +from pathlib import Path +from reboot.aio.backoff import Backoff +from reboot.cli.commands.dev import ( + check_local_envoy_mode, + try_and_become_child_subreaper_on_linux, +) +from reboot.cli.common import terminal +from reboot.cli.common.directories import ( + add_working_directory_options, + dot_rbt_directory, + use_working_directory, +) +from reboot.cli.common.rc import ArgumentParser +from reboot.cli.common.subprocesses import Subprocesses +from reboot.dashboard.constants import ( + DASHBOARD_PATH, + DEFAULT_DASHBOARD_PORT, + ENVVAR_RBT_API_DIRECTORY, +) +from reboot.settings import ( + ENVVAR_RBT_DEV, + ENVVAR_RBT_FRONTEND_DIST_PATH, + ENVVAR_RBT_FRONTEND_HOST, + ENVVAR_RBT_FRONTEND_ROOT_PATH, + ENVVAR_RBT_NAME, + ENVVAR_RBT_NODEJS, + ENVVAR_RBT_SERVERS, + ENVVAR_RBT_STATE_DIRECTORY, + ENVVAR_REBOOT_CRYPTO_ROOT_KEYS, + ENVVAR_REBOOT_EXPECTED_VERSION, + ENVVAR_REBOOT_LOCAL_ENVOY, + ENVVAR_REBOOT_LOCAL_ENVOY_PORT, + ENVVAR_REBOOT_OAUTH_SIGNING_SECRET, +) +from reboot.version import REBOOT_VERSION +from typing import Optional + +# The dashboard application's name, which names its state directory +# under `.rbt/`. A sibling of `.rbt/dev/` rather than inside it, so +# that it can never collide with a developer's application, whose +# state lives at `.rbt/dev//`. +DASHBOARD_STATE_DIRECTORY_NAME = 'dashboard' + + +def dashboard_subcommands() -> list[str]: + return ['dashboard'] + + +def register_dashboard(parser: ArgumentParser): + add_working_directory_options(parser.subcommand('dashboard')) + + parser.subcommand('dashboard').add_argument( + '--api-directory', + type=str, + required=True, + help='directory containing the API files the dashboard watches', + ) + + parser.subcommand('dashboard').add_argument( + '--port', + type=int, + help='port on which the dashboard will serve traffic; defaults to ' + f'{DEFAULT_DASHBOARD_PORT}', + ) + + +def _dashboard_env( + args, + parser: ArgumentParser, + *, + port: int, + api_directory: str, +) -> dict[str, str]: + """The environment for the dashboard application. + + Built from the ambient environment rather than from any + application environment, so that nothing naming a developer's + application, such as its name, state directory, port, launcher or + frontend, reaches an application that shares none of it. + """ + composed = os.environ.copy() + + # Every other application-flavored variable is overwritten below; + # these four have no dashboard value to overwrite them with, so a + # developer's shell export would leak through and make the + # dashboard a Node.js application or serve their frontend. + for name in ( + ENVVAR_RBT_NODEJS, + ENVVAR_RBT_FRONTEND_HOST, + ENVVAR_RBT_FRONTEND_DIST_PATH, + ENVVAR_RBT_FRONTEND_ROOT_PATH, + ): + composed.pop(name, None) + + composed[ENVVAR_RBT_DEV] = 'true' + composed[ENVVAR_REBOOT_EXPECTED_VERSION] = REBOOT_VERSION + composed[ENVVAR_REBOOT_LOCAL_ENVOY] = 'true' + composed[ENVVAR_REBOOT_LOCAL_ENVOY_PORT] = str(port) + + # A single server, so that a subscriber's `Connect` and + # `Toggle` always land on the same process; presence tracks its + # connections in memory there. `ENVVAR_REBOOT_LOCAL_ENVOY` is + # set above because one server otherwise turns Envoy off, and + # the browser has to reach this application. + composed[ENVVAR_RBT_SERVERS] = '1' + + # Where the developer's API files are, which the dashboard can + # read whether or not anything is running. Passed the way the + # developer spelled it, so that a file can be shown as + # `api/bank/v1/account.py`; the dashboard runs in this working + # directory, where that spelling resolves. + composed[ENVVAR_RBT_API_DIRECTORY] = api_directory + + composed[ENVVAR_RBT_NAME] = DASHBOARD_STATE_DIRECTORY_NAME + + state_directory = ( + dot_rbt_directory(args, parser) / DASHBOARD_STATE_DIRECTORY_NAME + ) + composed[ENVVAR_RBT_STATE_DIRECTORY] = str(state_directory) + + root_keys_path = state_directory / 'crypto-root-keys' + if root_keys_path.exists(): + composed[ENVVAR_REBOOT_CRYPTO_ROOT_KEYS] = root_keys_path.read_text() + else: + root_keys = f'v1:{secrets.token_urlsafe(32)}' + root_keys_path.parent.mkdir(parents=True, exist_ok=True) + root_keys_path.write_text(root_keys) + composed[ENVVAR_REBOOT_CRYPTO_ROOT_KEYS] = root_keys + + composed[ENVVAR_REBOOT_OAUTH_SIGNING_SECRET] = composed[ + ENVVAR_REBOOT_CRYPTO_ROOT_KEYS] + + return composed + + +async def _run_dashboard( + *, + env: dict[str, str], + state_directory: Path, + subprocesses: Subprocesses, +) -> None: + """Runs the dashboard application, restarting it if it exits. + + The dashboard's schema changes whenever Reboot's does, so the + expected reason for it to fail at startup is a backwards + incompatibility after an upgrade. Its state is ours and is + disposable, so the first failure deletes it and tries again + without asking. A second failure is something else, and gets + reported once rather than silently retried forever. + """ + backoff = Backoff() + failures = 0 + reported = False + + while True: + async with subprocesses.exec( + sys.executable, + '-m', + 'reboot.dashboard.main', + env=env, + # The application's own startup output would drown the one + # line this command prints; anything it writes to stderr + # still reaches the terminal. + stdout=asyncio.subprocess.DEVNULL, + ) as process: + await process.wait() + failed = process.returncode != 0 + + if not failed: + failures = 0 + else: + failures += 1 + + if failures == 1: + await asyncio.to_thread( + shutil.rmtree, state_directory, ignore_errors=True + ) + elif not reported: + reported = True + terminal.warn( + 'The dashboard application keeps failing to start; ' + 'still trying.' + ) + + await backoff() + + +async def dashboard( + args, + parser: ArgumentParser, +) -> int: + """Implementation of the 'dashboard' subcommand.""" + with use_working_directory(args, parser): + # If on Linux try to become a child subreaper so that we can + # properly clean up all processes descendant from us! Envoy in + # particular is a grandchild, and one that outlives the + # application would keep answering on the dashboard's port. + try_and_become_child_subreaper_on_linux() + + subprocesses = Subprocesses() + + # Pick the mode in which we'll run a local Envoy proxy and + # check that the mode is usable, e.g. that Docker is running + # and can access the Envoy proxy image, or that the `envoy` + # executable runs. Fail otherwise. + await check_local_envoy_mode(subprocesses) + + port = args.port or DEFAULT_DASHBOARD_PORT + + env = _dashboard_env( + args, + parser, + port=port, + api_directory=args.api_directory, + ) + + terminal.info( + 'Your dashboard is at ' + f'http://127.0.0.1:{port}{DASHBOARD_PATH}/\n' + ) + + await _run_dashboard( + env=env, + state_directory=Path(env[ENVVAR_RBT_STATE_DIRECTORY]), + subprocesses=subprocesses, + ) + + return 0 + + +async def handle_dashboard_subcommand( + args: argparse.Namespace, + *, + parser: ArgumentParser, +) -> Optional[int]: + if args.subcommand == 'dashboard': + return await dashboard(args, parser) + return None diff --git a/reboot/cli/commands/dev.py b/reboot/cli/commands/dev.py index 00f10e6a..45c1e2fc 100644 --- a/reboot/cli/commands/dev.py +++ b/reboot/cli/commands/dev.py @@ -12,6 +12,7 @@ import sys import termios import tty +import webbrowser from colorama import Fore from cryptography import x509 from cryptography.hazmat.backends import default_backend @@ -23,9 +24,12 @@ OTEL_EXPORTER_OTLP_TRACES_INSECURE, ) from pathlib import Path +from rbt.dashboard.v1.dashboard_rbt import Preferences +from rbt.std.presence.v1.presence_rbt import Presence from reboot.aio.backoff import Backoff from reboot.aio.contexts import EffectValidation from reboot.aio.exceptions import InputError +from reboot.aio.external import ExternalContext from reboot.cli.commands.generate import generate_direct # We import the whole `terminal` module (as opposed to the methods it contains) # to allow us to mock these methods out in tests. @@ -52,6 +56,12 @@ ) from reboot.cli.common.watch import FileWatcher, file_watcher from reboot.controller.plan_makers import validate_num_servers +from reboot.dashboard.constants import ( + DASHBOARD_PATH, + DEFAULT_DASHBOARD_PORT, + PREFERENCES_ID, + PRESENCE_ID, +) from reboot.server.local_envoy_factory import LocalEnvoyFactory from reboot.settings import ( DEFAULT_SECURE_PORT, @@ -233,6 +243,28 @@ def _register_dev_run(parser: ArgumentParser): f'{DEFAULT_LOCAL_ENVOY_PORT}', ) + parser.subcommand('dev run').add_argument( + '--open-dashboard', + type=bool, + # Three states, two of which currently agree: + # '--open-dashboard' opens one; unset and + # '--no-open-dashboard' both leave the browser alone while + # `_AUTO_OPEN_DASHBOARD` is off. Unset regains a meaning of + # its own once that is turned on: open one unless somebody + # is already looking at one, or the banner said not to. + default=None, + help='open a dashboard in your browser once your application ' + 'is serving', + ) + + parser.subcommand('dev run').add_argument( + '--dashboard-port', + type=int, + help='port on which the developer dashboard, started separately ' + f'with `rbt dashboard`, is serving; defaults to ' + f'{DEFAULT_DASHBOARD_PORT}', + ) + parser.subcommand('dev run').add_argument( '--watch', type=str, @@ -420,6 +452,138 @@ async def _run( application_started_event.clear() +async def _viewers(dashboard_url: str) -> list[str]: + """The subscriber ids of everyone looking at a dashboard. + + The dashboard constructs the `Presence` instance, empty, when it + initializes, so there is an answer from the moment it is up. + """ + context = ExternalContext(name="dev-run-open-dashboard", url=dashboard_url) + response = await Presence.ref(PRESENCE_ID).List(context) + return list(response.subscriber_ids) + + +async def _open_on_restart(dashboard_url: str) -> bool: + """Whether the developer still wants a dashboard opened for them. + + The dashboard's banner writes this when they click it, and it + outlives the `rbt dev run` they clicked it in. The dashboard + writes the default when it initializes, so nobody ever clicking + means a dashboard opens. + """ + context = ExternalContext(name="dev-run-open-dashboard", url=dashboard_url) + response = await Preferences.ref(PREFERENCES_ID).Get(context) + return not response.suppress_open_on_restart + + +# Whether `rbt dev run` may open a dashboard nobody asked it for. +# +# False while the dashboard is still being built, so that the only way +# to see one is `--open-dashboard`. Everything that decides when to +# open one by itself, the `Presence` subscribers and the banner's +# choice, is covered by `open_dashboard_tests`; turning auto-open on +# is this constant and nothing else. +_AUTO_OPEN_DASHBOARD = False + + +async def _open_dashboard_once( + *, + dashboard_url: str, + forced: bool, +) -> None: + """Opens a dashboard, unless the developer would rather it didn't. + + They would rather it didn't in two cases. One is that somebody is + already looking at one: the page subscribes to `Presence` for as + long as it is open, so a tab left up from an earlier run keeps a + second one from appearing, and a tab they closed is replaced. The + other is that they clicked the dashboard's "Don't reopen this + dashboard on restart" banner, which is remembered until they click + the banner that undoes it. + + `Presence` learns that a viewer has gone from the cancellation of + the page's `Connect` RPC, and nothing else. A proxy that holds its + server-side socket open after the browser goes away therefore + leaves a viewer listed who is not there, and the effect is that no + dashboard opens, which `--open-dashboard` overrides, as it + overrides the banner. + """ + if not forced: + if not await _open_on_restart(dashboard_url): + terminal.info( + 'You asked for this dashboard not to be reopened; run ' + 'with `--open-dashboard` to see it anyway, or visit ' + f'{dashboard_url}{DASHBOARD_PATH}/' + ) + return + + if len(await _viewers(dashboard_url)) > 0: + # Say why nothing opened, since a run that opens nothing + # and explains nothing is indistinguishable from a broken + # one, particularly when the tab being counted is behind + # another window, or on another screen. + terminal.info( + 'A dashboard is already open for this application; run ' + 'with `--open-dashboard` for another, or visit ' + f'{dashboard_url}{DASHBOARD_PATH}/' + ) + return + + # `webbrowser` honors `$BROWSER`, which is what makes this work in + # Codespaces and devcontainers, and returns `False` rather than + # raising when there is no browser to open. + page_url = f'{dashboard_url}{DASHBOARD_PATH}/' + + if not await asyncio.to_thread(webbrowser.open, page_url): + terminal.warn( + f"Could not open a browser; your dashboard is at {page_url}" + ) + + +async def _dashboard_reachable(port: int) -> bool: + """Whether something is accepting connections on the dashboard's + port.""" + try: + _, writer = await asyncio.wait_for( + asyncio.open_connection('127.0.0.1', port), + timeout=2.0, + ) + except (OSError, asyncio.TimeoutError): + return False + writer.close() + try: + await writer.wait_closed() + except OSError: + pass + return True + + +async def _open_dashboard( + *, + dashboard_url: str, + forced: bool, + application_serving_event: asyncio.Event, +) -> None: + """Opens a dashboard once the developer's application is serving + traffic, so that nothing the developer waits on is ever waiting on + this.""" + await application_serving_event.wait() + + try: + await _open_dashboard_once( + dashboard_url=dashboard_url, + forced=forced, + ) + except Exception as e: + # Never let this take down `rbt dev run`; the developer's + # application is unaffected and the dashboard is still + # reachable by hand. + terminal.warn( + f"Could not open a dashboard ({e}); it is at " + f"{dashboard_url}{DASHBOARD_PATH}/" + ) + + def try_and_become_child_subreaper_on_linux(): if sys.platform == 'linux': # The 'pyprctl' module is available on Linux only. @@ -542,6 +706,7 @@ async def _check_local_envoy_status( port: int, terminate_after_health_check: bool, application_started_event: asyncio.Event, + application_serving_event: Optional[asyncio.Event], tls_certificate: Optional[str], root_certificate: Optional[str], tracing: Tracing, @@ -650,6 +815,9 @@ def create_channel( was_application_serving = is_application_serving if is_application_serving: + if application_serving_event is not None: + application_serving_event.set() + terminal.info("Application is serving traffic ...\n") # MCP server and endpoint is not supported for Nodejs currently. mcp_line = ( @@ -1358,12 +1526,32 @@ async def __dev_run( await check_local_envoy_mode(subprocesses) env[ENVVAR_REBOOT_LOCAL_ENVOY] = 'true' + # The developer dashboard runs separately, started by + # `rbt dashboard`; this run only decides whether to open a window + # on it. + dashboard_port = args.dashboard_port or DEFAULT_DASHBOARD_PORT + + open_dashboard = ( + args.open_dashboard is True or + (_AUTO_OPEN_DASHBOARD and args.open_dashboard is not False) + ) + + if open_dashboard and not await _dashboard_reachable(dashboard_port): + terminal.fail( + 'You asked for a dashboard, but no developer dashboard is ' + f'serving on port {dashboard_port}. Start one with ' + '`rbt dashboard`, then run this again.' + ) + + application_serving_event = (asyncio.Event() if open_dashboard else None) + health_check_task = asyncio.create_task( _check_local_envoy_status( port=args.port or DEFAULT_LOCAL_ENVOY_PORT, terminate_after_health_check=args.terminate_after_health_check or False, application_started_event=application_started_event, + application_serving_event=application_serving_event, tls_certificate=args.tls_certificate, root_certificate=args.tls_root_certificate, tracing=tracing, @@ -1419,9 +1607,9 @@ def crypto_root_keys() -> str: protect, so that a JWT signed with them dies with the state it refers to (pushing clients back through the OAuth flow, whose fresh mint re-constructs per-user state): a named application - persists a random value in its state directory — stable across - restarts, deleted by `rbt dev expunge` and by the in-run `x` - expunge — while an anonymous application, whose state doesn't + persists a random value in its state directory, stable across + restarts and deleted by `rbt dev expunge` and by the in-run `x` + expunge, while an anonymous application, whose state doesn't survive a restart, gets fresh random keys on every (re)start. """ if args.application_name is None: @@ -1437,6 +1625,19 @@ def crypto_root_keys() -> str: root_keys_path.write_text(root_keys) return root_keys + if open_dashboard: + assert application_serving_event is not None + background_command_tasks.append( + asyncio.create_task( + _open_dashboard( + dashboard_url=f'http://127.0.0.1:{dashboard_port}', + forced=args.open_dashboard is True, + application_serving_event=application_serving_event, + ), + name=f'_open_dashboard(...) in {__name__}', + ) + ) + if tracing == Tracing.JAEGER: # TODO: dynamic port. See comment in `_run_jaeger()`. env[OTEL_EXPORTER_OTLP_TRACES_ENDPOINT] = "localhost:4317" diff --git a/reboot/cli/common/BUILD.bazel b/reboot/cli/common/BUILD.bazel index de53cec0..c15aae2b 100644 --- a/reboot/cli/common/BUILD.bazel +++ b/reboot/cli/common/BUILD.bazel @@ -145,6 +145,7 @@ py_library( deps = [ ":rc_py", ":update_check_py", + "//reboot/cli/commands:dashboard_py", "//reboot/cli/commands:dev_py", "//reboot/cli/commands:export_import_py", "//reboot/cli/commands:generate_py", diff --git a/reboot/cli/common/cli.py b/reboot/cli/common/cli.py index 5279e3b9..9d4dd7ab 100644 --- a/reboot/cli/common/cli.py +++ b/reboot/cli/common/cli.py @@ -7,6 +7,11 @@ handle_cloud_subcommand, register_cloud, ) +from reboot.cli.commands.dashboard import ( + dashboard_subcommands, + handle_dashboard_subcommand, + register_dashboard, +) from reboot.cli.commands.dev import ( dev_subcommands, handle_dev_subcommand, @@ -71,7 +76,7 @@ def create_parser( program='rbt', filename='.rbtrc', subcommands=( - cloud_subcommands() + dev_subcommands() + + cloud_subcommands() + dashboard_subcommands() + dev_subcommands() + export_and_import_subcommands() + generate_subcommands() + init_subcommands() + inspect_subcommands() + serve_subcommands() + task_subcommands() @@ -83,6 +88,7 @@ def create_parser( add_global_options(parser) register_cloud(parser) + register_dashboard(parser) register_dev(parser) register_export_and_import(parser) register_generate(parser) @@ -124,6 +130,10 @@ async def cli() -> int: if (result := await handle_cloud_subcommand(args)) is not None: return result + elif ( + result := await handle_dashboard_subcommand(args, parser=parser) + ) is not None: + return result elif ( result := await handle_dev_subcommand( args, diff --git a/reboot/dashboard/BUILD.bazel b/reboot/dashboard/BUILD.bazel new file mode 100644 index 00000000..4a7a1d0c --- /dev/null +++ b/reboot/dashboard/BUILD.bazel @@ -0,0 +1,120 @@ +load("@rbt_pypi//:requirements.bzl", "requirement") +load("@rules_python//python:defs.bzl", "py_binary", "py_library") + +py_library( + name = "constants_py", + srcs = ["constants.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], +) + +py_library( + name = "api_reader_py", + srcs = ["api_reader.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot:api_py", + ], +) + +py_library( + name = "api_watcher_py", + srcs = ["api_watcher.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":api_reader_py", + ":constants_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot/aio:external_py", + "//reboot/cli/common:watch_py", + ], +) + +py_library( + name = "servicers_py", + srcs = ["servicers.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":api_watcher_py", + ":constants_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot/aio:servicers_py", + "//reboot/std/presence/v1:presence_py", + ], +) + +# The page, its stylesheet and its bundle, served as three files by +# the `StaticFiles` mount in `main.py`. Only the script's entry point +# is rewritten: the one in the template, `./src/main.tsx`, is +# what Vite serves in development and does not exist in a built page, +# where `esbuild` produced `dashboard_bundle.js` instead. The +# stylesheet is copied as it is, because `./dashboard.css` resolves +# the same way in both. Every output must be *this* package's, because +# the application serves them from beside its own module rather than +# from a project root, and a `genrule` can only write within its own +# package. +genrule( + name = "dashboard_dist", + srcs = [ + "//reboot/dashboard/frontend:dashboard.css", + "//reboot/dashboard/frontend:dashboard_bundle.js", + "//reboot/dashboard/frontend:index.html", + ], + outs = [ + "dashboard/index.html", + "dashboard/dashboard.css", + "dashboard/dashboard_bundle.js", + ], + cmd = "sed 's|\\./src/main\\.tsx|./dashboard_bundle.js|' " + + "$(location //reboot/dashboard/frontend:index.html) " + + "> $(location dashboard/index.html) && " + + "cp $(location //reboot/dashboard/frontend:dashboard.css) " + + "$(location dashboard/dashboard.css) && " + + "cp $(location //reboot/dashboard/frontend:dashboard_bundle.js) " + + "$(location dashboard/dashboard_bundle.js)", +) + +py_library( + name = "main_py", + srcs = ["main.py"], + data = [ + ":dashboard_dist", + ], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":api_watcher_py", + ":constants_py", + ":servicers_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot/aio:applications_py", + "//reboot/aio:external_py", + "//reboot/aio:headers_py", + "//reboot/aio/backoff:python", + requirement("starlette"), + ], +) + +py_binary( + name = "main", + srcs = ["main.py"], + data = [ + ":dashboard_dist", + ], + main = "main.py", + visibility = ["//visibility:public"], + deps = [ + ":constants_py", + ":servicers_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot/aio:applications_py", + "//reboot/aio:external_py", + "//reboot/aio:headers_py", + "//reboot/aio/backoff:python", + requirement("starlette"), + ], +) diff --git a/reboot/dashboard/api_reader.py b/reboot/dashboard/api_reader.py new file mode 100644 index 00000000..dbf55816 --- /dev/null +++ b/reboot/dashboard/api_reader.py @@ -0,0 +1,202 @@ +"""Describes one of the developer's API files. + +Run as a subprocess: + + python -m reboot.dashboard.api_reader \ + + +and it writes a JSON list of `StateTypeInfo` to stdout, or a message +to stderr and a non-zero exit if the file cannot be read. + +A subprocess for two reasons. Reading a Pydantic API means importing +it, so doing it in the dashboard would accumulate stale modules across +edits. And it derives a module path from a relative filename, so it +needs a working directory and `sys.path` that the dashboard should not +adopt. + +The description comes from walking the imported `API` object itself, +so everything is spelled the way its author spelled it: method names +as `names_like_this`, field types as `int` or `Optional[str]`, and +errors by the names of the declared models. +""" +import asyncio +import importlib +import json +import os +import sys +import types +from google.protobuf.json_format import MessageToDict +from rbt.dashboard.v1.dashboard_pb2 import FieldInfo, MethodInfo, StateTypeInfo +from reboot.api import API, MethodModel, Model +from typing import Any, Literal, Optional, Union, get_args, get_origin + + +def _type_string(annotation) -> str: + """The source spelling of `annotation`, e.g. `Optional[str]`.""" + if annotation is type(None): + return 'None' + if annotation is Any: + return 'Any' + + origin = get_origin(annotation) + + if origin is Union or origin is types.UnionType: + arguments = get_args(annotation) + others = [a for a in arguments if a is not type(None)] + spelled = ', '.join(_type_string(a) for a in others) + if len(others) == len(arguments): + return f'Union[{spelled}]' + if len(others) == 1: + return f'Optional[{spelled}]' + return f'Optional[Union[{spelled}]]' + if origin is Literal: + return str(annotation).replace('typing.', '') + if origin is list: + (item,) = get_args(annotation) + return f'list[{_type_string(item)}]' + if origin is dict: + key, value = get_args(annotation) + return f'dict[{_type_string(key)}, {_type_string(value)}]' + if origin is None and isinstance(annotation, type): + return annotation.__name__ + return str(annotation).replace('typing.', '') + + +def _fields_of(model: type[Model]) -> list[FieldInfo]: + return [ + FieldInfo(name=name, type=_type_string(field.annotation)) + for name, field in model.model_fields.items() + ] + + +def _describe_method(method_name: str, spec: MethodModel) -> MethodInfo: + info = MethodInfo( + name=method_name, + kind=spec.kind.value, + factory=spec.factory, + mcp=spec.mcp is not None, + errors=[error.__name__ for error in spec.errors], + ) + + if spec.request is not None: + info.arguments.extend(_fields_of(spec.request)) + if spec.response is not None: + info.returns.extend(_fields_of(spec.response)) + if spec.description is not None: + info.description = spec.description + + return info + + +def describe(api_directory: str, filename: str) -> list[dict]: + """Describes the state types declared in one API file. + + State type names are qualified by the file's directory, the way + the generated code qualifies them: `shop/v1/shop.py` declaring + `Shop` yields `shop.v1.Shop`. + """ + # The path as the developer spelled it, joined before anything + # resolves it away: with `--api-directory=api` the file shows as + # `api/bank/v1/account.py`, the path they would open. + file = os.path.join(api_directory, filename) + + directory = os.path.abspath(api_directory) + os.chdir(directory) + sys.path.insert(0, directory) + + module = importlib.import_module( + filename.rsplit('.py', 1)[0].replace(os.sep, '.') + ) + + api = getattr(module, 'api', None) + if not isinstance(api, API): + # Not every file in the directory declares an API; one + # holding shared code simply has nothing to describe. + return [] + + package = os.path.dirname(filename).replace(os.sep, '.') + + described = [] + for type_name, type_obj in api.get_types().items(): + info = StateTypeInfo( + name=f'{package}.{type_name}', + file=file, + fields=_fields_of(type_obj.state), + ) + if type_obj.description is not None: + info.description = type_obj.description + + for method_name, spec in type_obj.methods.items(): + # A `UI` method has no RPC to call, so there is nothing + # to put in a method row for it. + if isinstance(spec, MethodModel): + info.methods.append(_describe_method(method_name, spec)) + + described.append(MessageToDict(info, preserving_proto_field_name=True)) + + return described + + +async def read( + api_directory: str, + filename: str, +) -> tuple[list[dict], Optional[str]]: + """Describes one API file in a subprocess. + + Returns the state types it declares, and a message when it could + not be read. A half-written file is the normal case while someone + is typing, and is worth showing rather than hiding. + """ + process = await asyncio.create_subprocess_exec( + sys.executable, + '-m', + # Not `__name__`, which is `__main__` when this module is the + # one being run. + 'reboot.dashboard.api_reader', + api_directory, + filename, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + # Reading an API file imports it, and an import writes + # `__pycache__` beside the source, inside the directory + # being watched, so the write is itself a change, and every + # edit costs a second pass over every file. It also leaves + # bytecode in the developer's tree that nothing else put + # there. + env={ + **os.environ, 'PYTHONDONTWRITEBYTECODE': '1' + }, + ) + out, errors = await process.communicate() + + if process.returncode != 0: + return [], errors.decode().strip() + + try: + return json.loads(out), None + except json.JSONDecodeError as e: + return [], f'Could not read the description: {e}' + + +def main() -> int: + if len(sys.argv) != 3: + print(f'usage: {sys.argv[0]} ', file=sys.stderr) + return 2 + + try: + print(json.dumps(describe(sys.argv[1], sys.argv[2]))) + except SystemExit: + # A malformed API can reach `fail()` inside `reboot.api`, + # which raises this after printing why. Being a subprocess, + # that is a message for the dashboard rather than the end of + # it. + return 1 + except Exception as e: + print(f'{type(e).__name__}: {e}', file=sys.stderr) + return 1 + + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/reboot/dashboard/api_watcher.py b/reboot/dashboard/api_watcher.py new file mode 100644 index 00000000..7646ce2a --- /dev/null +++ b/reboot/dashboard/api_watcher.py @@ -0,0 +1,190 @@ +"""Watches the developer's API files and updates what they declare. + +The dashboard may start before the application exists. In an agentic +flow the API files are written first, then generated code, then +servicers, then a build, then a running process, so asking the +application would say nothing for minutes. Reading the files says +something immediately, and says more with each file that lands. + +Per file, so that state types appear as they are written rather than +all at once at the end, and so that one file which does not parse, +the normal case while someone is typing, costs only its own types. +""" +from google.protobuf.json_format import ParseDict +from log.log import get_logger +from pathlib import Path +from rbt.dashboard.v1.dashboard_pb2 import StateTypeInfo +from rbt.dashboard.v1.dashboard_rbt import API +from reboot.aio.contexts import WorkflowContext +from reboot.cli.common.watch import file_watcher +from reboot.dashboard.api_reader import read +from reboot.dashboard.constants import API_ID +from typing import Optional +from watchdog.events import FileSystemEvent + +logger = get_logger(__name__) + +# Only Pydantic APIs can be read so far. `.proto` and `.ts` are the +# other two forms `rbt generate` accepts; both are static parses and +# neither is written yet. +API_GLOB = '**/*.py' + +# Suffixes of the files `rbt generate` writes, which it therefore +# skips on the way back in. The same three appear in +# `reboot/cli/commands/generate.py`, which decides what to generate +# from, and in `reboot/cli/commands/dev.py`, which decides what to +# watch. Keep the three lists in step. +GENERATED_SUFFIXES = ('_rbt.py', '_pb2.py', '_pb2_grpc.py') + + +def _files(api_directory: Path) -> list[str]: + """Every candidate API file, relative to `api_directory`. + + Every `.py` the developer wrote, which is the rule `rbt generate` + uses. Whether one of them declares an API is answered by reading + it: an API is a Python object, built when the module executes, so + no amount of looking at the text settles it. A file that declares + none costs one subprocess and describes nothing, and `rbt dev run` + is already importing all of these on every save to regenerate. + """ + return sorted( + str(path.relative_to(api_directory)) + for path in api_directory.glob(API_GLOB) + if not path.name.endswith(GENERATED_SUFFIXES) + ) + + +class _Descriptions: + """What each file last declared, and what went wrong reading it. + + Keyed by file so that a file which stops parsing keeps the types + it last had: blanking the dashboard on every keystroke would make + it unreadable exactly while it is being used. + """ + + def __init__(self) -> None: + self._state_types: dict[str, list[dict]] = {} + self._errors: dict[str, str] = {} + + def update( + self, + filename: str, + state_types: list[dict], + error: Optional[str], + ) -> None: + if error is None: + self._state_types[filename] = state_types + self._errors.pop(filename, None) + else: + self._errors[filename] = error + + def retain(self, filenames: set[str]) -> None: + """Forgets files that are no longer there.""" + for stored in list(self._state_types): + if stored not in filenames: + del self._state_types[stored] + for stored in list(self._errors): + if stored not in filenames: + del self._errors[stored] + + def state_types(self) -> list[StateTypeInfo]: + described = [] + for filename in sorted(self._state_types): + for state_type in self._state_types[filename]: + described.append(ParseDict(state_type, StateTypeInfo())) + return described + + def error(self) -> str: + return '\n'.join( + f'{filename}: {self._errors[filename]}' + for filename in sorted(self._errors) + ) + + +def _event_filenames(event: FileSystemEvent, directory: Path) -> set[str]: + """The filenames an event names, relative to `directory`. + + Both of its paths, because a rename reports where the file went as + well as where it was. A path that is not under the directory is + left out, and an event that names nothing under it is the caller's + signal that it could not place the event at all. + """ + filenames = set() + for path in (event.src_path, event.dest_path): + if not path: + continue + try: + filenames.add(str(Path(path).relative_to(directory))) + except ValueError: + continue + return filenames + + +async def watch(context: WorkflowContext, *, api_directory: str) -> None: + """Updates what the API files declare, for as long as this runs.""" + directory = Path(api_directory).resolve() + descriptions = _Descriptions() + updated: Optional[tuple] = None + + async def update_if_changed(alias: str) -> None: + nonlocal updated + + current = (descriptions.state_types(), descriptions.error()) + if current != updated: + updated = current + # Every write from a workflow needs an identity, and this + # one writes once per file that changed, on every + # iteration. + await API.ref(API_ID).per_iteration(alias).Update( + context, + state_types=descriptions.state_types(), + error=descriptions.error(), + ) + + # Everything, once: the developer may have written the whole API + # before the dashboard started. After this only what changes is + # read again. + previous_listing = set(_files(directory)) + pending = set(previous_listing) + + with file_watcher() as watcher: + async for iteration in context.loop('read what changed'): + # The watch is armed before anything is read, so a save + # made during a read is not missed: it resolves `event` + # rather than arriving while nothing is listening. A watch + # is consumed by one event, so it is re-entered for each, + # the same shape `rbt dev run` uses. + async with watcher.watch( + [API_GLOB], + root_dir=str(directory), + ) as event: + # Updating after each file rather than after the + # batch is what makes the types appear as they are + # written. + for filename in sorted(pending): + state_types, error = await read(api_directory, filename) + descriptions.update(filename, state_types, error) + await update_if_changed(f'read {filename}') + + changed = await event + + # A listing is a glob and no file reads, so it is taken on + # every change: it is what notices a file added or deleted, + # which an event naming one path cannot. + filenames = set(_files(directory)) + event_filenames = _event_filenames(changed, directory) + pending = ( + (filenames - previous_listing) | (event_filenames & filenames) + ) + previous_listing = filenames + + if not event_filenames: + # The glob only matches `.py` under this directory, so + # an event that names nothing under it means its paths + # did not resolve the way this one did. Read every file + # rather than let the page go quietly stale on a + # mismatch this cannot see. + pending = filenames + + descriptions.retain(filenames) + await update_if_changed('retain') diff --git a/reboot/dashboard/constants.py b/reboot/dashboard/constants.py new file mode 100644 index 00000000..5a16d3b9 --- /dev/null +++ b/reboot/dashboard/constants.py @@ -0,0 +1,46 @@ +"""Values shared between the dashboard application and the CLI. + +Kept apart from `main.py` so that reading them does not drag in the +application and everything it serves with. +""" + +# Where the dashboard application serves its page, relative to its +# own address. +DASHBOARD_PATH = '/dashboard' + +# The dashboard application's port. Deliberately not adjacent to +# `rbt dev run`'s default port of 9991: VS Code forwards a port +# upward when the one it wants is already taken on the developer's +# machine, so a second dev container serving on 9991 arrives on +# 9992. A dashboard sitting there could be reached in place of +# somebody else's backend, which half-works and is far more +# confusing than not working at all. 9871 is below 9991 so upward +# forwarding never reaches it, outside the 999x band (9990 k3d and +# WildFly, 9993 ZeroTier, 9997 Splunk), clear of +# 9000/9090/9200/9222/9229, hard to confuse with 9991 when reading +# logs, and outside the Linux ephemeral range. +DEFAULT_DASHBOARD_PORT = 9871 + +# The `API` state holding the shape the developer's API files +# declare, as the dashboard application last read them. +API_ID = 'api' + +# The `Preferences` state holding what the developer has said about +# opening dashboards: the dashboard's banner writes it and `rbt dev +# run` reads it. +PREFERENCES_ID = 'preferences' + +# The directory the developer's API files are in, which +# `rbt dashboard` takes as `--api-directory`. Separate from the +# application's URL because the files are there long before anything +# is serving, and the dashboard is meant to be startable that early. +ENVVAR_RBT_API_DIRECTORY = 'RBT_API_DIRECTORY' + +# The `Presence` state the dashboard page subscribes to, recording who +# is looking at a dashboard right now. `rbt dev run` reads it to decide +# whether to open one. +# +# The page names this, `API_ID` and `PREFERENCES_ID` independently, +# in `frontend/src/constants.ts`, since TypeScript cannot read them +# from here. Keep the two in step. +PRESENCE_ID = 'dashboard' diff --git a/reboot/dashboard/frontend/BUILD.bazel b/reboot/dashboard/frontend/BUILD.bazel new file mode 100644 index 00000000..6e5b62ed --- /dev/null +++ b/reboot/dashboard/frontend/BUILD.bazel @@ -0,0 +1,51 @@ +load("@aspect_rules_esbuild//esbuild:defs.bzl", "esbuild") +load("@aspect_rules_ts//ts:defs.bzl", "ts_config", "ts_project") + +ts_config( + name = "tsconfig", + src = "tsconfig.json", +) + +ts_project( + name = "dashboard_ts", + srcs = [ + "src/constants.ts", + "src/main.tsx", + ], + tsconfig = ":tsconfig", + deps = [ + "//:node_modules/@reboot-dev/reboot-react", + "//:node_modules/@reboot-dev/reboot-std", + "//:node_modules/@reboot-dev/reboot-std-api", + "//:node_modules/@reboot-dev/reboot-std-react", + "//:node_modules/@reboot-dev/reboot-web", + "//:node_modules/react", + "//:node_modules/react-dom", + "//:node_modules/uuid", + "//rbt/dashboard/v1:dashboard_js_reboot_react", + ], +) + +# The `tsconfig` is passed so the bundle uses the automatic JSX +# runtime; without it the bundle crashes with `React is not defined`. +esbuild( + name = "dashboard_bundle", + srcs = [ + ":dashboard_ts", + ], + bazel_sandbox_plugin = False, + entry_point = "src/main.js", + format = "esm", + output = "dashboard_bundle.js", + platform = "browser", + tsconfig = "tsconfig.json", + visibility = ["//reboot/dashboard:__pkg__"], +) + +exports_files( + [ + "dashboard.css", + "index.html", + ], + visibility = ["//reboot/dashboard:__pkg__"], +) diff --git a/reboot/dashboard/frontend/dashboard.css b/reboot/dashboard/frontend/dashboard.css new file mode 100644 index 00000000..f6dfb208 --- /dev/null +++ b/reboot/dashboard/frontend/dashboard.css @@ -0,0 +1,681 @@ +/* Tokens from the Reboot Cloud design system the mockup imports + (`_ds/reboot-cloud-design-system-.../_ds_bundle.css`), as HSL + triplets so they compose with `hsl(var(--token) / alpha)` the + way the source does. Light only: the system has `.dark:` + utilities but the design never opts in. */ +:root { + --background: 47 36% 95%; + --foreground: 211.1 71.7% 22.2%; + --card: 0 0% 100%; + --primary: 211 72% 22%; + --primary-foreground: 355.7 100% 97.3%; + --secondary: 115 49% 76%; + --muted: 240 4.8% 95.9%; + --muted-foreground: 240 3.8% 46.1%; + --accent: 166 47% 61%; + --destructive: 0 84.2% 60.2%; + --border: 240 5.9% 90%; + --radius: 0.5rem; + + /* Values the design uses directly, alongside the tokens. */ + --sidebar: 47 30% 92%; + --border-strong: 240 5.9% 86%; + --border-soft: 240 5.9% 94%; + --surface-sunken: 240 4.8% 97%; + --prose: 211 40% 30%; + --returns: 166 47% 33%; + --errors: 0 62% 45%; +} + +* { + box-sizing: border-box; +} + +html, +body, +#root { + height: 100%; + margin: 0; +} + +body { + font-family: ui-sans-serif, system-ui, sans-serif; + background: hsl(var(--background)); + color: hsl(var(--foreground)); + font-size: 15px; + line-height: 1.5; + -webkit-font-smoothing: antialiased; +} + +a { + color: hsl(var(--primary)); + text-decoration: none; +} +a:hover { + color: hsl(166 47% 41%); +} +::selection { + background: hsl(var(--accent) / 0.35); +} + +main { + max-width: 840px; + margin: 0 auto; + padding: 40px 48px; +} + +.muted { + color: hsl(var(--muted-foreground)); +} + +.error { + color: hsl(var(--errors)); + background: hsl(var(--destructive) / 0.08); + border: 1px solid hsl(var(--destructive) / 0.3); + border-radius: var(--radius); + padding: 12px 14px; + /* The reader subprocess's stderr, so line breaks are meaningful. */ + white-space: pre-wrap; + font-family: ui-monospace, Menlo, monospace; + font-size: 12px; +} + +/* The banner sits above whatever the page is showing, and the + rest of the page takes what is left. `min-height: 0` so the + sidebar and the document pane scroll themselves instead of + growing the page. */ +.app { + display: flex; + flex-direction: column; + height: 100%; +} + +.shell { + display: grid; + grid-template-columns: 250px 1fr; + flex: 1; + min-height: 0; + overflow: hidden; +} + +/* --- Banner --- */ + +.banner { + display: flex; + justify-content: center; + padding: 5px 20px; + background: hsl(var(--card)); + border-bottom: 1px solid hsl(var(--border)); +} + +/* A button, because it acts rather than navigates, dressed as a + link so it asks for as little of the page as it is worth. */ +.banner-link { + padding: 0; + border: none; + background: none; + font: inherit; + font-size: 11.5px; + color: hsl(var(--muted-foreground)); + text-decoration: underline; + text-underline-offset: 2px; + cursor: pointer; +} + +.banner-link:hover { + color: hsl(var(--foreground)); +} + +/* --- Sidebar --- */ + +nav { + background: hsl(var(--sidebar)); + border-right: 1px solid hsl(var(--border)); + overflow-y: auto; + padding: 14px 8px; +} + +.eyebrow { + font-family: ui-monospace, Menlo, monospace; + font-size: 10.5px; + letter-spacing: 0.08em; + text-transform: uppercase; + color: hsl(var(--muted-foreground)); +} + +nav > .eyebrow { + padding: 0 8px 8px; +} + +.namespace-head { + display: grid; + grid-template-columns: 10px 1fr auto; + align-items: center; + gap: 0 7px; + width: 100%; + padding: 6px 10px; + border: none; + border-radius: 5px; + background: transparent; + color: inherit; + font: inherit; + text-align: left; + cursor: pointer; +} + +.namespace-head:hover { + background: hsl(var(--card) / 0.6); +} + +.caret { + font-family: ui-monospace, Menlo, monospace; + font-size: 10px; + color: hsl(240 3.8% 55%); +} + +.namespace-name { + font-family: ui-monospace, Menlo, monospace; + font-size: 11.5px; + font-weight: 600; +} + +nav a { + display: grid; + grid-template-columns: 1fr auto; + align-items: center; + gap: 0 8px; + padding: 5px 10px 5px 34px; + border-radius: 5px; + color: inherit; +} + +nav a:hover { + background: hsl(var(--card) / 0.6); + color: inherit; +} + +.nav-name { + font-weight: 600; + font-size: 12.5px; +} + +/* The counts name what they count, since a namespace's number and a + state type's number sit in the same column and mean different + things. `nowrap` because the sidebar is narrow enough that "12 + state types" would otherwise break across two lines. */ +.nav-count { + font-family: ui-monospace, Menlo, monospace; + font-size: 10px; + color: hsl(var(--muted-foreground)); + white-space: nowrap; +} + +/* --- Document pane --- */ + +.pane { + overflow-y: auto; + background: hsl(var(--background)); +} + +header { + max-width: 840px; + margin: 0 auto; + padding: 40px 48px 28px; +} + +header h1 { + font-size: 34px; + font-weight: 650; + letter-spacing: -0.02em; + margin: 6px 0 0; +} + +.state-type { + max-width: 840px; + margin: 0 auto; + padding: 0 48px 48px; +} + +.state-type > .eyebrow:first-child { + display: block; +} + +.state-type { + border-top: 1px solid hsl(var(--border-strong)); + padding-top: 36px; +} + +.state-type h2 { + font-size: 28px; + font-weight: 650; + letter-spacing: -0.02em; + margin: 6px 0 4px; +} + +.file { + font-family: ui-monospace, Menlo, monospace; + font-size: 11.5px; + color: hsl(var(--muted-foreground)); +} + +.eyebrow.section { + margin: 26px 0 10px; +} + +/* --- Expanding a state type --- */ + +/* The heading and its count on the left, the button pushed to + the right edge. Aligned on the baseline rather than centred, + so the button's label sits on the same line as the 28px + heading beside it. When the row is too narrow to hold both, + the button wraps to its own line and `space-between` has + nothing left to spread. */ +.state-type-head { + display: flex; + align-items: baseline; + justify-content: space-between; + gap: 8px 20px; + flex-wrap: wrap; +} + +.state-type-heading { + display: flex; + align-items: baseline; + gap: 12px; + flex-wrap: wrap; + min-width: 0; +} + +/* Solid while closed and outlined while open, so the one that + has something left to offer is the one that draws the eye. */ +.expand-button { + display: inline-flex; + align-items: center; + flex: none; + gap: 8px; + padding: 9px 18px; + border: 1px solid hsl(var(--primary)); + border-radius: calc(var(--radius) - 2px); + background: hsl(var(--primary)); + color: hsl(var(--primary-foreground)); + box-shadow: 0 1px 2px hsl(240 10% 40% / 0.2); + font: inherit; + font-size: 13px; + font-weight: 600; + cursor: pointer; + user-select: none; +} + +.expand-button:hover { + opacity: 0.88; +} + +.expand-button:active { + transform: translateY(1px); +} + +.state-type.is-expanded .expand-button { + border-color: hsl(240 5.9% 78%); + background: hsl(var(--muted)); + color: hsl(var(--primary)); + box-shadow: none; +} + +.expand-button .caret { + font-family: ui-monospace, Menlo, monospace; + font-size: 11px; +} + +.summary-line { + font-family: ui-monospace, Menlo, monospace; + font-size: 11px; + color: hsl(240 3.8% 55%); +} + +.empty { + border: 1px dashed hsl(var(--border-strong)); + border-radius: calc(var(--radius) - 2px); + padding: 12px 14px; + font-size: 12.5px; + color: hsl(var(--muted-foreground)); + line-height: 1.5; +} + +.fields { + display: flex; + flex-direction: column; + gap: 8px; +} + +.field { + display: flex; + align-items: baseline; + gap: 10px; + border: 1px solid hsl(240 5.9% 92%); + border-radius: calc(var(--radius) - 2px); + padding: 10px 14px; + background: hsl(var(--card) / 0.6); +} + +.field-name { + font-family: ui-monospace, Menlo, monospace; + font-size: 12.5px; + font-weight: 600; +} + +.field-type { + font-family: ui-monospace, Menlo, monospace; + font-size: 11.5px; + color: hsl(var(--muted-foreground)); +} + +.methods { + display: flex; + flex-direction: column; + gap: 14px; +} + +.method { + border: 1px solid hsl(var(--border)); + border-radius: var(--radius); + background: hsl(var(--card)); + overflow: hidden; +} + +.method-head { + padding: 14px 18px 12px; +} + +.method-title { + display: flex; + align-items: center; + gap: 8px; + flex-wrap: wrap; +} + +/* Closed, the methods of one state type share a single set of + columns, so every kind pill starts at the same x and so does every + tag: the eye can run down the list and see which methods write and + which are reachable over MCP without reading any of them. Open, + this all reverts to the flex row above, where each row packs its + pills against its own name and the reader is looking at one method + rather than comparing several. + + `subgrid` passes the columns down from `.methods` through the card + and its head, so the tracks are sized once across every row rather + than per row. Each element is pinned to a column by name, because + `factory` and `mcp` are optional: auto-placement would slide a + method's `mcp` tag into the empty `factory` column and lose the + alignment that is the whole point. + + Behind `@supports` because a browser without subgrid drops that one + declaration and keeps the rest, which would leave the cards as + plain grids whose columns are sized per row: worse than the flex + row it replaced. Chrome has had subgrid since 117; anything older + keeps the flex row and simply does not line up. */ +@supports (grid-template-columns: subgrid) { + .state-type:not(.is-expanded) .methods { + display: grid; + grid-template-columns: max-content max-content 1fr; + /* The 14px is the gap between methods; the columns want the 8px + that `.method-title` uses when it is laid out as flex. */ + row-gap: 14px; + column-gap: 8px; + } + + .state-type:not(.is-expanded) .method, + .state-type:not(.is-expanded) .method-head, + .state-type:not(.is-expanded) .method-title { + display: grid; + grid-column: 1 / -1; + grid-template-columns: subgrid; + align-items: center; + } + + /* A scroll container cannot be a subgrid, so the columns would stop + propagating at the card. `.method` is one only because of the + `overflow: hidden` that clips the detail to the card's rounded + corners, and a closed method has no detail drawn to clip. */ + .state-type:not(.is-expanded) .method { + overflow: visible; + } + + .state-type:not(.is-expanded) .method-detail { + grid-column: 1 / -1; + } + + .state-type:not(.is-expanded) .method-name { + grid-column: 1; + } + + .state-type:not(.is-expanded) .kind { + grid-column: 2; + } + + .state-type:not(.is-expanded) .tags { + grid-column: 3; + } +} + +/* Whatever tags a method has, side by side. Taken out of the + layout when it has none, so an open section has no gap to an + empty box in its title's flex row. */ +.tags { + display: flex; + align-items: center; + gap: 8px; +} + +.tags:empty { + display: none; +} + +/* Two families of pill. A `kind` is what the method does to + state, tinted along a scale from a read that changes nothing + to a workflow that runs over time; every method has exactly + one. A `tag` is something else that is true of the method, + and is solid rather than tinted so that it never reads as a + fifth kind. Both have a 1px border so the two families sit + at the same height beside each other. + + The neutral base below is what a `saga`, or a method whose kind + is unset, gets, since only the four kinds that exist today have a + colour. */ +/* Both pills are one width with the label centred in it, so a + column of them reads as a column rather than as ragged text. + `min-width` rather than `width`: a label longer than any we + have today widens its own pill instead of being clipped. 92px + fits the longest, `transaction`. */ +.kind, +.tag { + display: inline-flex; + align-items: center; + justify-content: center; + min-width: 92px; +} + +.kind { + font-family: ui-monospace, Menlo, monospace; + font-size: 10px; + padding: 2px 8px; + border-radius: 999px; + background: hsl(var(--muted)); + border: 1px solid hsl(240 5.9% 84%); + color: hsl(var(--muted-foreground)); +} + +/* Teal: reads state and changes nothing. */ +.kind-reader { + background: hsl(166 47% 61% / 0.18); + border-color: hsl(166 40% 55% / 0.45); + color: hsl(166 55% 27%); +} + +/* Blue: writes the one state it is called on. */ +.kind-writer { + background: hsl(211 72% 45% / 0.14); + border-color: hsl(211 60% 50% / 0.4); + color: hsl(211 72% 32%); +} + +/* Violet: writes across states, atomically. */ +.kind-transaction { + background: hsl(275 55% 55% / 0.14); + border-color: hsl(275 45% 55% / 0.4); + color: hsl(275 50% 40%); +} + +/* Amber: runs past the call that started it. */ +.kind-workflow { + background: hsl(32 90% 55% / 0.18); + border-color: hsl(32 75% 50% / 0.42); + color: hsl(28 80% 33%); +} + +.tag { + font-family: ui-monospace, Menlo, monospace; + font-size: 9px; + font-weight: 600; + padding: 2px 7px; + border: 1px solid transparent; + border-radius: 999px; + color: hsl(0 0% 100%); +} + +/* Navy: brings the state into existence. */ +.tag-factory { + background: hsl(211 72% 26%); +} + +/* Magenta: reachable by an agent over MCP. */ +.tag-mcp { + background: hsl(318 55% 38%); +} + +/* A pill that carries a definition. The mark is the invitation to + hover; the tooltip is the answer. `cursor: help` says the same + thing the mark does, for whoever reads cursors first. */ +.defined { + position: relative; + cursor: help; +} + +/* Small and translucent so it reads as an aside rather than as part + of the label. */ +.define-mark { + margin-left: 4px; + font-size: 8px; + opacity: 0.55; +} + +/* Above the pill, centred, and inert to the mouse so that moving + toward it never flickers it away. The pill styles it sits in are + undone piece by piece: pills are bold, tight and centred, and a + sentence is none of those. */ +.definition { + position: absolute; + bottom: calc(100% + 8px); + left: 50%; + transform: translateX(-50%); + width: max-content; + max-width: 260px; + padding: 8px 10px; + border-radius: 6px; + background: hsl(var(--primary)); + color: hsl(var(--primary-foreground)); + font-family: ui-sans-serif, system-ui, sans-serif; + font-size: 11.5px; + font-weight: 400; + line-height: 1.45; + letter-spacing: normal; + text-align: left; + text-transform: none; + white-space: normal; + pointer-events: none; + opacity: 0; + visibility: hidden; + transition: opacity 120ms ease; + z-index: 10; +} + +.defined:hover .definition { + opacity: 1; + visibility: visible; +} + +/* The section eyebrow sits at the pane's left edge, and the pane + clips whatever leaves it, so a centred tooltip loses its left + half. Open rightward from the label instead. */ +.eyebrow .definition { + left: 0; + transform: none; +} + +/* The height animation. A grid row interpolates from `0fr` to + `1fr`, which is how the detail can grow to whatever height it + happens to need without anything measuring it first, since there + is no height to read while it is closed. The inner element + owns `overflow: hidden`, because a grid row can only shrink + below its content when the content itself is willing to be + clipped. + + Opacity is on a shorter, later curve than the height so that + text fades in against a box that has already begun to open, + rather than appearing at full strength in a 1px slot. */ +.method-detail { + display: grid; + grid-template-rows: 0fr; + opacity: 0; + transition: grid-template-rows 240ms cubic-bezier(0.32, 0.72, 0, 1), + opacity 120ms ease-out; +} + +.method-detail-inner { + overflow: hidden; +} + +.state-type.is-expanded .method-detail { + grid-template-rows: 1fr; + opacity: 1; + transition: grid-template-rows 240ms cubic-bezier(0.32, 0.72, 0, 1), + opacity 180ms ease-in 60ms; +} + +@media (prefers-reduced-motion: reduce) { + .method-detail, + .state-type.is-expanded .method-detail { + transition-duration: 1ms; + } +} + +.method-description { + font-size: 13px; + line-height: 1.55; + color: hsl(var(--prose)); + margin: 0; + padding: 0 18px 12px; + text-wrap: pretty; +} + +.method-signature { + display: flex; + align-items: baseline; + gap: 12px; + flex-wrap: wrap; + padding: 10px 18px; + background: hsl(var(--surface-sunken)); + border-top: 1px solid hsl(var(--border-soft)); + font-family: ui-monospace, Menlo, monospace; + font-size: 11.5px; + color: hsl(var(--muted-foreground)); +} + +.arrow { + color: hsl(240 3.8% 65%); +} +.returns { + color: hsl(var(--returns)); +} +.errors { + color: hsl(var(--errors)); +} diff --git a/reboot/dashboard/frontend/index.html b/reboot/dashboard/frontend/index.html new file mode 100644 index 00000000..cb6c81d1 --- /dev/null +++ b/reboot/dashboard/frontend/index.html @@ -0,0 +1,18 @@ + + + + + + + Reboot dashboard + + + +
+ + + diff --git a/reboot/dashboard/frontend/src/constants.ts b/reboot/dashboard/frontend/src/constants.ts new file mode 100644 index 00000000..c8f515b8 --- /dev/null +++ b/reboot/dashboard/frontend/src/constants.ts @@ -0,0 +1,5 @@ +// Mirrors `reboot/dashboard/constants.py`, which +// TypeScript cannot read. Keep the two in step. +export const PRESENCE_ID = "dashboard"; +export const API_ID = "api"; +export const PREFERENCES_ID = "preferences"; diff --git a/reboot/dashboard/frontend/src/main.tsx b/reboot/dashboard/frontend/src/main.tsx new file mode 100644 index 00000000..b6fa0ecf --- /dev/null +++ b/reboot/dashboard/frontend/src/main.tsx @@ -0,0 +1,527 @@ +import type { MethodInfo, StateTypeInfo } from "@dashboard/dashboard_pb"; +import { useAPI, usePreferences } from "@dashboard/dashboard_rbt_react"; +import { RebootClientProvider } from "@reboot-dev/reboot-react"; +import { Presence } from "@reboot-dev/reboot-std-react/presence"; +import { + FC, + StrictMode, + useCallback, + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, +} from "react"; +import { createRoot } from "react-dom/client"; +import { v4 as uuidv4 } from "uuid"; +import { API_ID, PREFERENCES_ID, PRESENCE_ID } from "./constants"; + +// One subscriber per tab, for as long as the tab is open. +const SUBSCRIBER_ID = uuidv4(); + +// What each pill means, for somebody meeting Reboot for the first +// time. A pill whose word is not here, such as a kind this page +// does not know, simply gets no mark and no tooltip. +const DEFINITIONS: Record = { + reader: + "Reads state without changing it, so any number can safely " + + "execute concurrently. A reactive caller keeps receiving fresh " + + "results as the state changes.", + writer: + "Changes this state. Writers on one state run one at a time, " + + "each seeing the result of the one before it.", + transaction: + "Changes state, and can call methods on other states with " + + "every change landing together or none of them landing at all.", + workflow: + "A durable background task. It can loop and wait for as long " + + "as it needs, and after a restart it resumes where it was.", + factory: + "Brings a state into existence: it is called with a new id " + + "rather than on a state that already exists.", + mcp: "Callable by AI agents as a tool, over the Model Context " + "Protocol.", + "state type": + "A durable data type. Each instance, named by an id, has fields " + + "that Reboot persists for you. Methods are the way to read and " + + "change them. You can have as many of these as you want.", +}; + +// A pill, with its definition a hover away when it has one. The +// small mark is what says there is something to hover. +const Pill: FC<{ className: string; label: string; meaning?: string }> = ({ + className, + label, + meaning, +}) => + meaning === undefined ? ( + {label} + ) : ( + + {label} + + + {meaning} + + + ); + +const Kind: FC<{ kind: string }> = ({ kind }) => ( + +); + +// A state type's namespace is its proto package: `bank.v1.Account` +// lives in `bank.v1`, which is the developer's `api/bank/v1/`. +const namespaceOf = (name: string): string => + name.slice(0, name.lastIndexOf(".")); + +const typeNameOf = (name: string): string => + name.slice(name.lastIndexOf(".") + 1); + +// Standard-library types an application uses are real and worth being +// able to inspect, but they aren't what the developer wrote, so they +// start collapsed. +const isStandardLibrary = (namespace: string): boolean => + namespace.startsWith("rbt."); + +const Namespace: FC<{ namespace: string; types: StateTypeInfo[] }> = ({ + namespace, + types, +}) => { + const [open, setOpen] = useState(!isStandardLibrary(namespace)); + + return ( +
+ + {open && ( + + )} +
+ ); +}; + +const Method: FC<{ method: MethodInfo }> = ({ method }) => { + const args = method.arguments + .map((argument) => `${argument.name}: ${argument.type}`) + .join(", "); + + // The response's keys and value types, spelled the way a Python + // reader would write them. + const returns = method.returns + .map((field) => `${field.name}: ${field.type}`) + .join(", "); + + return ( +
+
+
+ {method.name} + {/* The kind first, and always: every method has one, so it + lands in the same place in every row and the eye can run + down the column. The tags after it are the exceptions. */} + + {/* One cell for whichever tags a method has, rather than a + column each: both are optional, so a column each would + make every method with neither hold that width open + as dead space. A method that is both a factory and + an MCP tool draws both, side by side. */} + + {method.factory && ( + + )} + {method.mcp && ( + + )} + +
+
+ {/* What the method's own row grows to show. Kept mounted while + the section is closed, because the animation that opens it + is a CSS transition on this element rather than a mount. */} +
+
+ {method.description !== undefined && ( +

{method.description}

+ )} +
+ + ({args}) →{" "} + + {method.returns.length > 0 ? `{${returns}}` : "None"} + + + {method.errors.length > 0 && ( + raises {method.errors.join(", ")} + )} +
+
+
+
+ ); +}; + +const countOf = (n: number, noun: string): string => + `${n} ${n === 1 ? noun : `${noun}s`}`; + +// Horizontal only, and deliberately. A pill's sideways move, between +// the column it shares while closed and its own row while open, is a +// layout change that CSS cannot transition, so it is animated here: +// measure where each pill was, let the layout happen, animate it from +// there. Its vertical move is not ours to animate. The detail growing +// is what pushes the methods below it down, and that already animates +// over the same 240ms, so translating them as well would move them +// twice and they would appear to fly in from above or below. +// +// `offsetLeft` rather than `getBoundingClientRect()` because it is a +// layout position and ignores transforms: a render that lands while a +// pill is mid-slide reads where it is going rather than where it +// momentarily is, so the next toggle starts from the truth. +const SLIDE_MS = 240; +const SLIDE_EASING = "cubic-bezier(0.32, 0.72, 0, 1)"; + +const useSlidingPills = (expanded: boolean) => { + const section = useRef(null); + const before = useRef(new WeakMap()); + const wasExpanded = useRef(expanded); + + // No dependency list: every render re-measures, so the positions + // this animates from are the ones on screen rather than the ones + // from the last toggle, which a window resize would have moved. + useLayoutEffect(() => { + const pills = section.current?.querySelectorAll(".kind, .tag"); + if (pills === undefined) { + return; + } + + // Only opening or closing moves them; other renders just leave + // fresh measurements behind for the next one that does. + const toggled = wasExpanded.current !== expanded; + wasExpanded.current = expanded; + + const still = window.matchMedia("(prefers-reduced-motion: reduce)").matches; + + // `forEach` rather than `for...of`: a `NodeList`'s iterator is + // typed as `Node`, which has no box to measure, while its + // `forEach` keeps the element type the selector asked for. + pills.forEach((pill) => { + const was = before.current.get(pill); + const now = pill.offsetLeft; + before.current.set(pill, now); + + if (!toggled || still || was === undefined || was === now) { + return; + } + + pill.animate( + [{ transform: `translateX(${was - now}px)` }, { transform: "none" }], + { duration: SLIDE_MS, easing: SLIDE_EASING } + ); + }); + }); + + return section; +}; + +const StateType: FC<{ + stateType: StateTypeInfo; + expanded: boolean; + onToggle: () => void; +}> = ({ stateType, expanded, onToggle }) => { + const section = useSlidingPills(expanded); + + return ( + // Every method's detail opens and closes off this one class, so a + // section is one transition rather than one per method. +
+
+ +
+
+
+

{typeNameOf(stateType.name)}

+ + {countOf(stateType.fields.length, "field")} ·{" "} + {countOf(stateType.methods.length, "method")} + +
+ +
+
{stateType.file}
+ +
state
+ {stateType.fields.length === 0 ? ( +
+ No state fields. The key is the whole state. +
+ ) : ( +
+ {stateType.fields.map((field) => ( +
+ {field.name} + {field.type} +
+ ))} +
+ )} + +
methods
+
+ {stateType.methods.map((method) => ( + + ))} +
+
+ ); +}; + +// Whether `rbt dev run` may open a dashboard by itself, and the one +// click that changes the answer. +const Banner: FC<{ suppressed: boolean; onToggle: () => void }> = ({ + suppressed, + onToggle, +}) => ( +
+ +
+); + +const Overview: FC<{ + isExpanded: (name: string) => boolean; + onToggle: (name: string) => void; +}> = ({ isExpanded, onToggle }) => { + // The dashboard's own state: what it read of the developer's API + // files. Nothing here reaches the application, so the application + // does not have to exist. + const { useGet } = useAPI({ id: API_ID }); + const { response, isLoading } = useGet(); + + // What the developer's API files declare. Those exist before the + // application is generated, built or started, which is why they are + // what this page shows. + const read = response?.stateTypes; + + // Restarting `rbt dashboard` closes this page's connection for a + // few seconds. Keep the last shape that was read so the page stays + // readable across that. + const seen = useRef([]); + + if (read !== undefined && read.length > 0) { + seen.current = read; + } + + const stateTypes: StateTypeInfo[] = read?.length ? read : seen.current; + + // Why the API files could not be read, shown beside the last shape + // that was: a half-written file is the normal case while someone is + // typing, and saying so beats showing nothing. + const error = response?.error ?? ""; + + const namespaces = useMemo(() => { + const byNamespace = new Map(); + for (const stateType of stateTypes) { + const namespace = namespaceOf(stateType.name); + const types = byNamespace.get(namespace); + if (types === undefined) { + byNamespace.set(namespace, [stateType]); + } else { + types.push(stateType); + } + } + // The developer's own namespaces first; the standard library is + // theirs to use but not theirs to read. + return [...byNamespace.entries()] + .map(([namespace, types]) => ({ namespace, types })) + .sort((a, b) => { + const standard = + Number(isStandardLibrary(a.namespace)) - + Number(isStandardLibrary(b.namespace)); + return standard !== 0 + ? standard + : a.namespace.localeCompare(b.namespace); + }); + }, [stateTypes]); + + // Only before anything has ever been read; afterwards the last + // shape is shown instead. + if (isLoading && stateTypes.length === 0) { + return ( +
+

Reboot application

+

Reading your API…

+
+ ); + } + + if (stateTypes.length === 0) { + return ( +
+

Reboot application

+

+ Waiting for your API. Nothing in your API directory declares state + types yet. +

+ {error &&
{error}
} +
+ ); + } + + return ( +
+ +
+
+
application domain
+

+ {stateTypes.length} state types in {namespaces.length}{" "} + {namespaces.length === 1 ? "namespace" : "namespaces"} +

+
+ {error &&
{error}
} + {stateTypes.map((stateType) => ( + onToggle(stateType.name)} + key={stateType.name} + /> + ))} +
+
+ ); +}; + +// Everything the developer has told this dashboard, in one place. +// Both choices are the dashboard application's state rather than +// this page's, so they survive the tab, the hot reload and the +// `rbt dev run` they were made in. +const App: FC = () => { + const { useGet, setSuppressOpenOnRestart, setExpanded } = usePreferences({ + id: PREFERENCES_ID, + }); + const { response } = useGet(); + + // Until the read lands, say what the CLI does when nothing has been + // written, which is the same thing it does on a false field. + const suppressed = response?.suppressOpenOnRestart ?? false; + + const stored = useMemo( + () => new Set(response?.expandedStateTypes ?? []), + [response?.expandedStateTypes] + ); + + // A click that has not yet come back from the application, standing + // in for the read until it does. Without it a section would sit + // still for a whole round trip after being clicked, which reads as + // a dead button rather than as a slow one. + const [clicked, setClicked] = useState(new Map()); + + // Drop each stand-in once the read agrees with it, so that a later + // change from another tab is followed rather than held off forever. + useEffect(() => { + setClicked((clicked) => { + const waiting = new Map( + [...clicked].filter(([name, expanded]) => stored.has(name) !== expanded) + ); + return waiting.size === clicked.size ? clicked : waiting; + }); + }, [stored]); + + const isExpanded = useCallback( + (name: string): boolean => clicked.get(name) ?? stored.has(name), + [clicked, stored] + ); + + const onToggle = useCallback( + (name: string): void => { + const expanded = !isExpanded(name); + setClicked((clicked) => new Map(clicked).set(name, expanded)); + setExpanded({ stateType: name, expanded }); + }, + [isExpanded, setExpanded] + ); + + return ( +
+ + setSuppressOpenOnRestart({ suppressOpenOnRestart: !suppressed }) + } + /> + +
+ ); +}; + +const root = document.getElementById("root"); + +if (root !== null) { + createRoot(root).render( + + {/* No `url`: the page and its presence are served by the same + application, so the client uses this page's origin. */} + + + + + + + ); +} diff --git a/reboot/dashboard/frontend/tsconfig.json b/reboot/dashboard/frontend/tsconfig.json new file mode 100644 index 00000000..3446f50f --- /dev/null +++ b/reboot/dashboard/frontend/tsconfig.json @@ -0,0 +1,16 @@ +{ + "compilerOptions": { + "target": "es2018", + "module": "esnext", + "jsx": "react-jsx", + "moduleResolution": "bundler", + "skipLibCheck": true, + "verbatimModuleSyntax": true, + "baseUrl": ".", + "paths": { + // The generated bindings for the dashboard's own state, which + // `dashboard_js_reboot_react` emits beside its proto. + "@dashboard/*": ["../../../rbt/dashboard/v1/*"] + } + } +} diff --git a/reboot/dashboard/main.py b/reboot/dashboard/main.py new file mode 100644 index 00000000..fdfc9e1b --- /dev/null +++ b/reboot/dashboard/main.py @@ -0,0 +1,84 @@ +"""The developer dashboard application. + +A Reboot application owned by the framework, holding the state the +dashboard needs but which must not be written into the +application under development, and serving the dashboard's page. It +is not part of the Reboot API and nothing imports it; it runs as its +own process, with its own state store, alongside the application +being developed. +""" +import asyncio +from pathlib import Path +from rbt.dashboard.v1.dashboard_rbt import API, Preferences +from rbt.std.presence.v1.presence_rbt import Presence +from reboot.aio.applications import Application +from reboot.aio.external import InitializeContext +from reboot.dashboard.constants import ( + API_ID, + DASHBOARD_PATH, + PREFERENCES_ID, + PRESENCE_ID, +) +from reboot.dashboard.servicers import servicers +from starlette.staticfiles import StaticFiles + +# The built page, beside this module, which is the same arrangement +# `InspectServicer` uses for its own assets. Mounting it directly +# avoids `RBT_FRONTEND_DIST_PATH`, which resolves against a project +# root discovered by walking up from a servicer's file; the servicers +# here come from `reboot.std.presence`, so no such root exists above +# them. +_DASHBOARD_DIRECTORY = Path(__file__).parent / 'dashboard' + + +def application() -> Application: + """The dashboard application, with its page mounted.""" + application = Application( + servicers=servicers(), + initialize=initialize, + ) + + application.http.mount( + DASHBOARD_PATH, + app=StaticFiles( + directory=str(_DASHBOARD_DIRECTORY), + # `html=True` so the directory URL serves `index.html`. + # `check_dir=False` so a not-yet-built page doesn't stop + # the application starting. `follow_symlink=True` because + # under Bazel runfiles the built page is a symlink into + # `bazel-out`, which Starlette's default `realpath` check + # rejects as escaping the served directory. + html=True, + check_dir=False, + follow_symlink=True, + ), + ) + + return application + + +async def initialize(context: InitializeContext) -> None: + """Gives `Preferences` the answer somebody who has never clicked + its banner should get.""" + await Preferences.ref(PREFERENCES_ID).SetSuppressOpenOnRestart( + context, + suppress_open_on_restart=False, + ) + + # Construct the `Presence` instance, empty, so that a read of + # who is looking at a dashboard has an answer from the moment the + # dashboard is up. + await Presence.ref(PRESENCE_ID).Create(context) + + # Idempotently, so that a restart of a named application finds + # the `Watch` it already spawned rather than starting a second + # watcher. + _ = await API.ref(API_ID).idempotently('watch').spawn().Watch(context) + + +async def main(): + await application().run() + + +if __name__ == '__main__': + asyncio.run(main()) diff --git a/reboot/dashboard/servicers.py b/reboot/dashboard/servicers.py new file mode 100644 index 00000000..dec9cd41 --- /dev/null +++ b/reboot/dashboard/servicers.py @@ -0,0 +1,131 @@ +"""Servicers for the developer dashboard application.""" +import os +import reboot.std.presence.v1.presence +from rbt.dashboard.v1.dashboard_pb2 import ( + APIGetRequest, + APIGetResponse, + APIUpdateRequest, + APIUpdateResponse, + PreferencesGetRequest, + PreferencesGetResponse, + PreferencesSetExpandedRequest, + PreferencesSetExpandedResponse, + PreferencesSetSuppressOpenOnRestartRequest, + PreferencesSetSuppressOpenOnRestartResponse, +) +from rbt.dashboard.v1.dashboard_rbt import API, Preferences +from reboot.aio.auth.authorizers import allow +from reboot.aio.contexts import ReaderContext, WorkflowContext, WriterContext +from reboot.aio.servicers import Servicer +from reboot.dashboard.api_watcher import watch +from reboot.dashboard.constants import ENVVAR_RBT_API_DIRECTORY + + +class APIServicer(API.Servicer): + """Holds the shape the developer's API files declare.""" + + def authorizer(self): + return allow() + + async def Get( + self, + context: ReaderContext, + request: APIGetRequest, + ) -> APIGetResponse: + return APIGetResponse( + state_types=self.state.state_types, + error=self.state.error, + ) + + @classmethod + async def Watch( + cls, + context: WorkflowContext, + request: API.WatchRequest, + ) -> API.WatchResponse: + """Reads the developer's API files when they change. + + The directory comes from the environment each time this runs, + so that an `rbt dashboard` restarted against a different one + reads the new directory. Taking it from the request would keep + whichever directory the run that started watching had. + """ + api_directory = os.environ[ENVVAR_RBT_API_DIRECTORY] + + await watch(context, api_directory=api_directory) + + return API.WatchResponse() + + async def Update( + self, + context: WriterContext, + request: APIUpdateRequest, + ) -> APIUpdateResponse: + del self.state.state_types[:] + self.state.state_types.extend(request.state_types) + self.state.error = request.error + return APIUpdateResponse() + + +class PreferencesServicer(Preferences.Servicer): + """Holds what the developer has said about their dashboard. + + Two unrelated choices share one state because both are facts about + this machine's dashboard rather than about the application, and + each has its own writer so that recording one never overwrites the + other. + """ + + def authorizer(self): + return allow() + + async def Get( + self, + context: ReaderContext, + request: PreferencesGetRequest, + ) -> PreferencesGetResponse: + return PreferencesGetResponse( + suppress_open_on_restart=self.state.suppress_open_on_restart, + expanded_state_types=self.state.expanded_state_types, + ) + + async def SetSuppressOpenOnRestart( + self, + context: WriterContext, + request: PreferencesSetSuppressOpenOnRestartRequest, + ) -> PreferencesSetSuppressOpenOnRestartResponse: + self.state.suppress_open_on_restart = request.suppress_open_on_restart + return PreferencesSetSuppressOpenOnRestartResponse() + + async def SetExpanded( + self, + context: WriterContext, + request: PreferencesSetExpandedRequest, + ) -> PreferencesSetExpandedResponse: + expanded = set(self.state.expanded_state_types) + + if request.expanded: + expanded.add(request.state_type) + else: + expanded.discard(request.state_type) + + self.state.expanded_state_types[:] = sorted(expanded) + + return PreferencesSetExpandedResponse() + + +def servicers() -> list[type[Servicer]]: + """The servicers that back the dashboard's own state. + + This state belongs to the dashboard rather than to the application + being developed, so it lives in its own application and its own + state store. + + This is a library rather than something built into the application + below it, so that what these servicers are stays separate from what + ends up hosting them. + """ + return [ + APIServicer, + PreferencesServicer, + ] + reboot.std.presence.v1.presence.servicers() diff --git a/reboot/inspect/BUILD.bazel b/reboot/inspect/BUILD.bazel index 007e4e00..5083da6d 100644 --- a/reboot/inspect/BUILD.bazel +++ b/reboot/inspect/BUILD.bazel @@ -49,6 +49,19 @@ esbuild( platform = "browser", ) +py_library( + name = "describe_state_type_py", + srcs = ["describe_state_type.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + "//log:log_py", + "//rbt/v1alpha1:options_py_proto", + "//rbt/v1alpha1/inspect:inspect_py_proto", + "//reboot/aio:types_py", + ], +) + py_library( name = "servicer_py", srcs = ["servicer.py"], diff --git a/reboot/inspect/describe_state_type.py b/reboot/inspect/describe_state_type.py new file mode 100644 index 00000000..62bbe43e --- /dev/null +++ b/reboot/inspect/describe_state_type.py @@ -0,0 +1,141 @@ +"""Describes Reboot state types from protobuf descriptors. + +Everything comes from the descriptor pool it is handed: a state type's +fields, and the methods of the services that supply them. Field and +method types are rendered as the Python names a person reads on a +page, such as `int` and `list[str]`, rather than as their protobuf +spelling. +""" +from google.protobuf import descriptor_pool +from google.protobuf.descriptor import FieldDescriptor +from log.log import get_logger +from rbt.v1alpha1 import options_pb2 +from rbt.v1alpha1.inspect.inspect_pb2 import ( + FieldInfo, + MethodInfo, + StateTypeInfo, +) +from reboot.aio.types import StateTypeName +from typing import Iterable, Optional + +logger = get_logger(__name__) + +_TYPE_NAMES = { + FieldDescriptor.TYPE_DOUBLE: 'float', + FieldDescriptor.TYPE_FLOAT: 'float', + FieldDescriptor.TYPE_INT64: 'int', + FieldDescriptor.TYPE_UINT64: 'int', + FieldDescriptor.TYPE_INT32: 'int', + FieldDescriptor.TYPE_FIXED64: 'int', + FieldDescriptor.TYPE_FIXED32: 'int', + FieldDescriptor.TYPE_BOOL: 'bool', + FieldDescriptor.TYPE_STRING: 'str', + FieldDescriptor.TYPE_BYTES: 'bytes', + FieldDescriptor.TYPE_UINT32: 'int', + FieldDescriptor.TYPE_SFIXED32: 'int', + FieldDescriptor.TYPE_SFIXED64: 'int', + FieldDescriptor.TYPE_SINT32: 'int', + FieldDescriptor.TYPE_SINT64: 'int', +} + + +def _type_name(field) -> str: + """How to render `field`'s type.""" + if field.type in ( + FieldDescriptor.TYPE_MESSAGE, FieldDescriptor.TYPE_GROUP + ): + name = field.message_type.name + elif field.type == FieldDescriptor.TYPE_ENUM: + name = field.enum_type.name + else: + name = _TYPE_NAMES.get(field.type, 'unknown') + + if field.label == FieldDescriptor.LABEL_REPEATED: + return f'list[{name}]' + return name + + +def _fields_of(message) -> list[FieldInfo]: + return [ + FieldInfo(name=field.name, type=_type_name(field)) + for field in message.fields + ] + + +def _describe_method(method) -> MethodInfo: + options = method.GetOptions().Extensions[options_pb2.method] + kind = options.WhichOneof('kind') or '' + + info = MethodInfo( + name=method.name, + kind=kind, + arguments=_fields_of(method.input_type), + errors=list(options.errors), + mcp=options.HasField('mcp'), + ) + + # An empty response means the method returns nothing; saying + # "Empty" would be an implementation detail leaking out. The + # response's fields rather than its name, because a synthesized + # name such as `ShopRemainingResponse` says nothing the fields + # don't. + if method.output_type.full_name != 'google.protobuf.Empty': + info.returns.extend(_fields_of(method.output_type)) + + # An application that was created before `MethodOptions.description` + # will have the deprecated `mcp` description, which is permitted + # for backward compatibility. + if options.description: + info.description = options.description + elif options.HasField('mcp') and options.mcp.description: + info.description = options.mcp.description + + # Only writers and transactions can construct. + if kind in ('writer', 'transaction'): + info.factory = getattr(options, kind).HasField('constructor') + + return info + + +def describe_state_type( + pool: descriptor_pool.DescriptorPool, + state_type_name: StateTypeName, + service_names: Iterable[str], + file: str, +) -> Optional[StateTypeInfo]: + """Describes one state type, or `None` when its descriptors can't + be found, since a state type we can't describe shouldn't stop us + describing the rest. + + `pool` holds the state type's descriptors and those of the + services named in `service_names`, which supply its methods. + + `file` is the file the developer declared the state type in, which + is reported as it is. A Pydantic API is described from a `.proto` + synthesized from it, so the descriptors name a file that only + exists inside the build. + """ + try: + state = pool.FindMessageTypeByName(state_type_name) + except KeyError: + logger.warning( + f"No descriptor for state type '{state_type_name}'; " + "omitting it from the schema" + ) + return None + + info = StateTypeInfo( + name=state_type_name, + file=file, + fields=_fields_of(state), + ) + + for service_name in service_names: + try: + service = pool.FindServiceByName(service_name) + except KeyError: + continue + for method in service.methods: + info.methods.append(_describe_method(method)) + + return info diff --git a/reboot/std/presence/v1/presence.py b/reboot/std/presence/v1/presence.py index 69025d14..86740dc1 100644 --- a/reboot/std/presence/v1/presence.py +++ b/reboot/std/presence/v1/presence.py @@ -19,6 +19,7 @@ WaitForDisconnectRequest, WaitForDisconnectResponse, ) +from rbt.std.presence.v1 import presence_rbt from rbt.std.presence.v1.presence_rbt import ( ListRequest, ListResponse, @@ -67,6 +68,14 @@ class PresenceServicer(Presence.singleton.Servicer): def authorizer(self): return allow() + async def Create( + self, + context: WriterContext, + state: Presence.State, + request: presence_rbt.CreateRequest, + ) -> presence_rbt.CreateResponse: + return presence_rbt.CreateResponse() + async def Subscribe( self, context: WriterContext, diff --git a/tests/reboot/cli/BUILD.bazel b/tests/reboot/cli/BUILD.bazel index 0c03443d..fcbac08f 100644 --- a/tests/reboot/cli/BUILD.bazel +++ b/tests/reboot/cli/BUILD.bazel @@ -33,6 +33,20 @@ py_test( ], ) +py_test( + name = "dashboard_tests_py", + srcs = [ + "dashboard_tests.py", + ], + main = "dashboard_tests.py", + deps = [ + ":mock_exit_py", + "//reboot/cli/common:cli_py", + "//reboot/cli/common:rc_py", + "//reboot/dashboard:constants_py", + ], +) + py_test( name = "dev_tests_py", srcs = [ diff --git a/tests/reboot/cli/dashboard_tests.py b/tests/reboot/cli/dashboard_tests.py new file mode 100644 index 00000000..f5bea4ff --- /dev/null +++ b/tests/reboot/cli/dashboard_tests.py @@ -0,0 +1,127 @@ +import os +import tempfile +import unittest +from reboot.cli.commands import dashboard +from reboot.cli.common import cli +from reboot.cli.common.directories import dot_rbt_directory +from reboot.cli.common.rc import ArgumentParser +from reboot.dashboard.constants import DEFAULT_DASHBOARD_PORT +from tests.reboot.cli.mock_exit import ( + MockExitException, + mock_raise_instead_of_exit, +) +from unittest.mock import patch + + +@patch('argparse.ArgumentParser.exit', mock_raise_instead_of_exit) +class RbtDashboardTestCase(unittest.IsolatedAsyncioTestCase): + + def _parse(self, state_directory: str): + parser: ArgumentParser = cli.create_parser( + argv=[ + 'rbt', + f'--state-directory={state_directory}', + 'dashboard', + '--api-directory=api', + ] + ) + args, _ = parser.parse_args() + return args, parser + + async def test_api_directory_is_required(self) -> None: + parser: ArgumentParser = cli.create_parser(argv=['rbt', 'dashboard']) + with self.assertRaises(MockExitException): + parser.parse_args() + + async def test_env_is_isolated_from_any_application(self) -> None: + with tempfile.TemporaryDirectory() as state_directory: + args, parser = self._parse(state_directory) + + # Values naming a developer's application must not survive + # into the dashboard's environment; if any did, the + # dashboard would collide with their state directory or + # port. + with patch.dict( + os.environ, + { + 'RBT_NAME': 'app', + 'RBT_STATE_DIRECTORY': '/somewhere/app', + 'RBT_NODEJS': 'true', + 'REBOOT_LOCAL_ENVOY_PORT': '9991', + }, + ): + env = dashboard._dashboard_env( + args, + parser, + port=DEFAULT_DASHBOARD_PORT, + api_directory=args.api_directory, + ) + + self.assertEqual(env['RBT_NAME'], 'dashboard') + self.assertNotIn('RBT_NODEJS', env) + self.assertEqual( + env['REBOOT_LOCAL_ENVOY_PORT'], + str(DEFAULT_DASHBOARD_PORT), + ) + + # One server, and Envoy explicitly on: one server would + # otherwise turn Envoy off, and the browser has to reach + # the dashboard. + self.assertEqual(env['RBT_SERVERS'], '1') + self.assertEqual(env['REBOOT_LOCAL_ENVOY'], 'true') + + # A sibling of `.rbt/dev/`, so that it can never collide + # with an application's state at `.rbt/dev//`. + self.assertEqual( + env['RBT_STATE_DIRECTORY'], + str(dot_rbt_directory(args, parser) / 'dashboard'), + ) + + async def test_keys_differ_from_any_application(self) -> None: + with tempfile.TemporaryDirectory() as state_directory: + args, parser = self._parse(state_directory) + + with patch.dict( + os.environ, {'REBOOT_CRYPTO_ROOT_KEYS': 'v1:theirs'} + ): + env = dashboard._dashboard_env( + args, + parser, + port=DEFAULT_DASHBOARD_PORT, + api_directory=args.api_directory, + ) + + self.assertNotEqual(env['REBOOT_CRYPTO_ROOT_KEYS'], 'v1:theirs') + + # Stable across restarts, so tokens the dashboard mints + # stay valid until its state is deleted. + again = dashboard._dashboard_env( + args, + parser, + port=DEFAULT_DASHBOARD_PORT, + api_directory=args.api_directory, + ) + self.assertEqual( + env['REBOOT_CRYPTO_ROOT_KEYS'], + again['REBOOT_CRYPTO_ROOT_KEYS'], + ) + + async def test_is_told_where_the_api_files_are(self) -> None: + with tempfile.TemporaryDirectory() as state_directory: + args, parser = self._parse(state_directory) + + env = dashboard._dashboard_env( + args, + parser, + port=DEFAULT_DASHBOARD_PORT, + api_directory=args.api_directory, + ) + + # As the developer spelled it, so files can be shown as + # `api/bank/v1/account.py`; the dashboard runs in the + # working directory where that spelling resolves. + self.assertEqual(env['RBT_API_DIRECTORY'], 'api') + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/reboot/cli/dev_tests.py b/tests/reboot/cli/dev_tests.py index 80785e37..1993842a 100644 --- a/tests/reboot/cli/dev_tests.py +++ b/tests/reboot/cli/dev_tests.py @@ -119,6 +119,22 @@ async def test_environment_variable(self) -> None: [['E1', 'V1'], ['E2', 'V2'], ['E3', 'V3']], ) + async def test_open_dashboard_requires_a_reachable_dashboard(self) -> None: + # `rbt dev run --open-dashboard` refuses to start when nothing + # is serving on the dashboard's port, telling the developer to + # run `rbt dashboard`. + server = await asyncio.start_server( + lambda reader, writer: writer.close(), '127.0.0.1', 0 + ) + port = server.sockets[0].getsockname()[1] + + self.assertTrue(await dev._dashboard_reachable(port)) + + server.close() + await server.wait_closed() + + self.assertFalse(await dev._dashboard_reachable(port)) + async def test_dev_expunge_requires_name(self) -> None: parser: ArgumentParser = cli.create_parser( argv=[ diff --git a/tests/reboot/dashboard/BUILD.bazel b/tests/reboot/dashboard/BUILD.bazel new file mode 100644 index 00000000..c9956e23 --- /dev/null +++ b/tests/reboot/dashboard/BUILD.bazel @@ -0,0 +1,83 @@ +load("@rules_python//python:defs.bzl", "py_test") +load("//tests/reboot/react:py_web_test_suite_env.bzl", "py_web_test_suite_env") + +py_test( + name = "api_reader_tests_py", + srcs = ["api_reader_tests.py"], + data = glob(["api/**"]), + main = "api_reader_tests.py", + deps = [ + "//reboot/dashboard:api_reader_py", + ], +) + +py_test( + name = "api_watcher_tests_py", + srcs = ["api_watcher_tests.py"], + main = "api_watcher_tests.py", + deps = [ + "//reboot/aio:tests_py", + "//reboot/dashboard:api_watcher_py", + "//reboot/dashboard:main_py", + ], +) + +py_test( + name = "application_tests_py", + srcs = [":application_tests.py"], + main = "application_tests.py", + deps = [ + "//reboot/aio:tests_py", + "//reboot/dashboard:servicers_py", + "//reboot/std/presence/v1:presence_py", + ], +) + +py_web_test_suite_env( + name = "dashboard_tests_py", + srcs = ["dashboard_tests.py"], + browsers = [ + "@io_bazel_rules_webtesting//browsers:chromium-local", + ], + main = "dashboard_tests.py", + py_test_tags = [ + "macos_not_supported", + "requires-linux-x86", + ], + tags = [ + "macos_not_supported", + "requires-linux-x86", + ], + deps = [ + "//reboot/aio:tests_py", + "//reboot/dashboard:api_watcher_py", + "//reboot/dashboard:main_py", + "//reboot/std/presence/v1:presence_py", + "@io_bazel_rules_webtesting//testing/web", + ], +) + +py_test( + name = "preferences_tests_py", + srcs = ["preferences_tests.py"], + main = "preferences_tests.py", + deps = [ + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot/aio:tests_py", + "//reboot/dashboard:main_py", + ], +) + +py_test( + name = "open_dashboard_tests_py", + srcs = ["open_dashboard_tests.py"], + main = "open_dashboard_tests.py", + deps = [ + "//rbt/dashboard/v1:dashboard_py_reboot", + "//rbt/std/presence/subscriber/v1:subscriber_py_reboot", + "//rbt/std/presence/v1:presence_py_reboot", + "//reboot/aio:tests_py", + "//reboot/cli/commands:dev_py", + "//reboot/dashboard:main_py", + ], +) diff --git a/tests/reboot/dashboard/api/shop/v1/helper.py b/tests/reboot/dashboard/api/shop/v1/helper.py new file mode 100644 index 00000000..0c75c3a7 --- /dev/null +++ b/tests/reboot/dashboard/api/shop/v1/helper.py @@ -0,0 +1,3 @@ +"""Shared code beside an API file, with no `api` of its own.""" + +TAX = 0.1 diff --git a/tests/reboot/dashboard/api/shop/v1/shop.py b/tests/reboot/dashboard/api/shop/v1/shop.py new file mode 100644 index 00000000..f49b264d --- /dev/null +++ b/tests/reboot/dashboard/api/shop/v1/shop.py @@ -0,0 +1,48 @@ +from reboot.api import ( + API, + Field, + Methods, + Model, + Reader, + Tool, + Transaction, + Type, +) + + +class ShopState(Model): + name: str = Field(tag=1) + open: bool = Field(tag=2) + + +class StockRequest(Model): + item: str = Field(tag=1) + quantity: int = Field(tag=2) + + +class StockResponse(Model): + remaining: int = Field(tag=1) + + +class OutOfStockError(Model): + item: str = Field(tag=1) + + +ShopMethods = Methods( + create=Transaction(request=None, response=None, factory=True, mcp=None), + stock=Transaction( + request=StockRequest, + response=None, + description="Add stock of an item.", + mcp=None, + ), + remaining=Reader( + request=StockRequest, + response=StockResponse, + errors=[OutOfStockError], + description="How much of an item is left.", + mcp=Tool(), + ), +) + +api = API(Shop=Type(state=ShopState, methods=ShopMethods)) diff --git a/tests/reboot/dashboard/api_reader_tests.py b/tests/reboot/dashboard/api_reader_tests.py new file mode 100644 index 00000000..b5d3d23e --- /dev/null +++ b/tests/reboot/dashboard/api_reader_tests.py @@ -0,0 +1,127 @@ +"""The dashboard describes an API file without the application. + +This is what lets the dashboard show state types before anything has +been built: `rbt generate` has not run, no servicer exists, and there +is no process to ask. Only the file the developer wrote. +""" +import os +import tempfile +import unittest +from pathlib import Path +from reboot.dashboard.api_reader import read + +API_DIRECTORY = str(Path(__file__).parent / 'api') + + +def _by_name(state_types: list[dict]) -> dict[str, dict]: + return {state_type['name']: state_type for state_type in state_types} + + +def _method(state_type: dict, name: str) -> dict: + for method in state_type['methods']: + if method['name'] == name: + return method + raise AssertionError(f"No method '{name}' in {state_type['name']}") + + +class APIReaderTest(unittest.IsolatedAsyncioTestCase): + + async def test_describes_a_state_type_and_its_methods(self) -> None: + state_types, error = await read(API_DIRECTORY, 'shop/v1/shop.py') + + self.assertIsNone(error) + + shop = _by_name(state_types)['shop.v1.Shop'] + + # The file the developer wrote, spelled from where the + # dashboard was started: the API directory as given, then the + # path inside it. + self.assertEqual( + shop['file'], + os.path.join(API_DIRECTORY, 'shop/v1/shop.py'), + ) + + self.assertEqual( + [field['name'] for field in shop['fields']], + ['name', 'open'], + ) + + # The methods come from the file, with the names and kinds + # their author wrote. + stock = _method(shop, 'stock') + self.assertEqual(stock['kind'], 'transaction') + self.assertEqual( + [argument['name'] for argument in stock['arguments']], + ['item', 'quantity'], + ) + + # `stock` has a `description` and is not an MCP tool: prose + # reaches the page whether or not its author also exposed the + # method to MCP. + self.assertEqual(stock['description'], 'Add stock of an item.') + self.assertNotIn('mcp', stock) + + remaining = _method(shop, 'remaining') + self.assertEqual(remaining['kind'], 'reader') + self.assertEqual( + remaining['returns'], + [{ + 'name': 'remaining', + 'type': 'int', + }], + ) + self.assertTrue(remaining['mcp']) + + # The errors a method declares, by the names of the declared + # models. + self.assertEqual(remaining['errors'], ['OutOfStockError']) + self.assertEqual( + remaining['description'], + 'How much of an item is left.', + ) + + # A factory constructs the state, and returns nothing. + create = _method(shop, 'create') + self.assertTrue(create['factory']) + self.assertNotIn('returns', create) + + async def test_a_file_with_no_api_describes_nothing(self) -> None: + # A directory holds shared code as well as APIs, and reading a + # module that declares no `api` is not an error. + state_types, error = await read(API_DIRECTORY, 'shop/v1/helper.py') + + self.assertIsNone(error) + self.assertEqual(state_types, []) + + async def test_a_file_that_does_not_parse_reports_why(self) -> None: + # Half-written files are the normal case while someone is + # typing. The reader has to survive them and say what is + # wrong, because that message is what the developer needs. + with tempfile.TemporaryDirectory() as directory: + os.makedirs(os.path.join(directory, 'shop', 'v1')) + Path(os.path.join(directory, 'shop', 'v1', 'shop.py') + ).write_text('from reboot.api import API\napi = API(\n') + + state_types, error = await read(directory, 'shop/v1/shop.py') + + self.assertEqual(state_types, []) + assert error is not None + self.assertIn('SyntaxError', error) + + async def test_reading_does_not_write_to_the_developer_s_tree( + self + ) -> None: + # Reading walks the API object in memory and leaves the + # developer's tree exactly as it was. + before = sorted(os.listdir(os.path.join(API_DIRECTORY, 'shop', 'v1'))) + + await read(API_DIRECTORY, 'shop/v1/shop.py') + + self.assertEqual( + before, + sorted(os.listdir(os.path.join(API_DIRECTORY, 'shop', 'v1'))), + ) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/reboot/dashboard/api_watcher_tests.py b/tests/reboot/dashboard/api_watcher_tests.py new file mode 100644 index 00000000..e5fa6b84 --- /dev/null +++ b/tests/reboot/dashboard/api_watcher_tests.py @@ -0,0 +1,111 @@ +"""State types appear as the developer writes their API files. + +The dashboard is up before the application exists, so this is the +first thing a dashboard can show: not what is running, but what has +been written so far. +""" +import asyncio +import os +import tempfile +import unittest +from pathlib import Path +from rbt.dashboard.v1.dashboard_rbt import API +from reboot.aio.tests import Reboot +from reboot.dashboard.constants import API_ID, ENVVAR_RBT_API_DIRECTORY +from reboot.dashboard.main import application +from typing import Optional +from unittest.mock import patch + +SHOP = ''' +from reboot.api import API, Field, Methods, Model, Reader, Type + + +class {state}State(Model): + name: str = Field(tag=1) + + +class LookRequest(Model): + item: str = Field(tag=1) + + +class LookResponse(Model): + found: bool = Field(tag=1) + + +{state}Methods = Methods( + look=Reader( + request=LookRequest, + response=LookResponse, + description=None, + mcp=None, + ), +) + +api = API({state}=Type(state={state}State, methods={state}Methods)) +''' + + +class APIWatcherTest(unittest.IsolatedAsyncioTestCase): + + watcher: Optional[asyncio.Task] = None + + async def asyncSetUp(self) -> None: + # The workflow reads the directory when the application comes + # up, so it has to exist and be named first. + self._directory = tempfile.TemporaryDirectory() + self.directory = Path(self._directory.name) + self._environment = patch.dict( + os.environ, + {ENVVAR_RBT_API_DIRECTORY: str(self.directory)}, + ) + self._environment.start() + + self.rbt = Reboot() + await self.rbt.start() + await self.rbt.up(application(), local_envoy=True) + self.url = f'http://127.0.0.1:{self.rbt.envoy_port()}' + + async def asyncTearDown(self) -> None: + await self.rbt.stop() + self._environment.stop() + self._directory.cleanup() + + def _write(self, directory: Path, name: str, state: str) -> None: + path = directory / 'shop' / 'v1' / f'{name}.py' + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(SHOP.format(state=state)) + + async def _wait_for(self, satisfied): + while True: + context = self.rbt.create_external_context(name=self.id()) + try: + response = await API.ref(API_ID).Get(context) + if satisfied(response): + return response + except Exception: + pass + await asyncio.sleep(0.1) + + async def test_types_appear_as_files_are_written(self) -> None: + # The workflow is already watching: it was scheduled when the + # application came up. + self._write(self.directory, 'shop', 'Shop') + + response = await self._wait_for(lambda api: len(api.state_types) == 1) + self.assertEqual( + [state.name for state in response.state_types], + ['shop.v1.Shop'], + ) + self.assertEqual(response.error, '') + + self._write(self.directory, 'depot', 'Depot') + + response = await self._wait_for(lambda api: len(api.state_types) == 2) + self.assertEqual( + sorted(state.name for state in response.state_types), + ['shop.v1.Depot', 'shop.v1.Shop'], + ) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/reboot/dashboard/application_tests.py b/tests/reboot/dashboard/application_tests.py new file mode 100644 index 00000000..64ca0c13 --- /dev/null +++ b/tests/reboot/dashboard/application_tests.py @@ -0,0 +1,102 @@ +import asyncio +import unittest +from rbt.v1alpha1.errors_pb2 import NotFound, StateNotConstructed +from reboot.aio.applications import Application +from reboot.aio.external import ExternalContext +from reboot.aio.tests import Reboot +from reboot.dashboard.servicers import servicers +from reboot.std.presence.v1.presence import Presence, Subscriber + + +class TestDashboardApplication(unittest.IsolatedAsyncioTestCase): + """Checks that the dashboard application stands up on its own and + that presence works against it, which is the whole reason it + exists.""" + + async def asyncSetUp(self) -> None: + self.rbt = Reboot() + await self.rbt.start() + await self.rbt.up(Application(servicers=servicers())) + + async def asyncTearDown(self) -> None: + await self.rbt.stop() + + async def make_connection( + self, + presence_ref: Presence.WeakReference, + subscriber_ref: Subscriber.WeakReference, + context: ExternalContext, + nonce: str, + ) -> asyncio.Task: + """Connects `subscriber_ref` and subscribes it to `presence_ref`. + + `Toggle` is retried because it races `Connect`, which is what + registers the connection; until that has happened `Toggle` + reports `NotFound`. Returns the task running `Connect`, which + stays pending for as long as the subscriber is present. + """ + await subscriber_ref.idempotently().Create(context) + + connect_failed = False + + async def connect(): + nonlocal connect_failed + try: + await subscriber_ref.Connect(context, nonce=nonce) + except: + connect_failed = True + + connect_task = asyncio.create_task(connect()) + + attempt = 0 + while not connect_failed: + try: + await subscriber_ref.idempotently( + f"Attempt {attempt}", + ).Toggle(context, nonce=nonce) + except Subscriber.ToggleAborted as aborted: + if isinstance(aborted.error, NotFound): + attempt += 1 + continue + raise + + await presence_ref.Subscribe( + context, subscriber_id=subscriber_ref.state_id + ) + break + + return connect_task + + async def test_presence_reports_a_connected_subscriber(self) -> None: + context = self.rbt.create_external_context(name=f"test-{self.id()}") + + presence = Presence.ref("dashboard") + subscriber = Subscriber.ref("a-dashboard-tab") + + # Until a writer constructs the `Presence` state, `List` + # aborts rather than reporting an empty list. The dashboard's + # `initialize` calls `Create` for exactly this reason, so its + # readers always have an instance to read; this application has + # no `initialize`, which is what lets this case be seen. + with self.assertRaises(Presence.ListAborted) as aborted: + await presence.List(context) + self.assertIsInstance(aborted.exception.error, StateNotConstructed) + + connect_task = await self.make_connection( + presence, subscriber, context, nonce="nonce" + ) + + response = await presence.List(context) + self.assertEqual(list(response.subscriber_ids), [subscriber.state_id]) + + # Cancelling `Connect` is what a closing browser tab does, and + # the subscriber must drain back out again. + connect_task.cancel() + + async for response in presence.reactively().List(context): + if list(response.subscriber_ids) == []: + break + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/reboot/dashboard/dashboard_tests.py b/tests/reboot/dashboard/dashboard_tests.py new file mode 100644 index 00000000..67b1f6c7 --- /dev/null +++ b/tests/reboot/dashboard/dashboard_tests.py @@ -0,0 +1,418 @@ +"""The dashboard application serves its page, and that page describes +the application under development. + +It is itself a Reboot application, so it can describe itself: pointing +the page at its own address exercises the whole path (config route, +API read, and rendering) in one process. +""" +import asyncio +import socket +import unittest +from rbt.dashboard.v1.dashboard_pb2 import FieldInfo, MethodInfo, StateTypeInfo +from rbt.dashboard.v1.dashboard_rbt import API, Preferences +from reboot.aio.tests import Reboot +from reboot.dashboard.constants import ( + API_ID, + DASHBOARD_PATH, + PREFERENCES_ID, + PRESENCE_ID, +) +from reboot.dashboard.main import application +from reboot.std.presence.v1.presence import Presence +from selenium.webdriver.common.by import By +from selenium.webdriver.support import expected_conditions +from selenium.webdriver.support.wait import WebDriverWait +from testing.web import webtest + + +def _driver(): + return webtest.new_webdriver_session( + capabilities={ + 'goog:chromeOptions': + { + 'args': + [ + '--headless', + '--no-sandbox', + '--disable-dev-shm-usage', + ], + }, + 'goog:loggingPrefs': { + 'browser': 'ALL', + }, + } + ) + + +class DashboardTest(unittest.IsolatedAsyncioTestCase): + + async def asyncSetUp(self) -> None: + # The page is served by the dashboard application, so the + # test needs its + # address to point a browser at it. + with socket.socket() as probe: + probe.bind(('127.0.0.1', 0)) + port = probe.getsockname()[1] + + self.url = f'http://127.0.0.1:{port}' + + self.rbt = Reboot() + await self.rbt.start() + await self.rbt.up( + application(), + local_envoy=True, + local_envoy_port=port, + ) + + async def asyncTearDown(self) -> None: + await self.rbt.stop() + + async def _wait_for_viewers(self, satisfied, driver=None) -> None: + """Polls until presence satisfies `satisfied`. + + Polling rather than reading reactively because `List` aborts + with `StateNotConstructed` until somebody has subscribed at + least once, which is where a fresh application starts. + + Reports what it sees as it goes, including anything the page + logged. This wait has no deadline, so when it does not finish + the test is killed with its `finally` unrun. Printing only at + the end would mean printing nothing in the one case worth + explaining. + """ + polls = 0 + while True: + context = self.rbt.create_external_context(name=self.id()) + # A fresh reference per context; one cannot be shared. + presence = Presence.ref(PRESENCE_ID) + viewers: list[str] = [] + try: + response = await presence.List(context) + viewers = list(response.subscriber_ids) + if satisfied(viewers): + return + except Presence.ListAborted: + if satisfied([]): + return + + polls += 1 + if polls % 10 == 0: + print(f'##### still waiting, {polls} polls, viewers={viewers}') + if driver is not None: + for entry in await asyncio.to_thread( + driver.get_log, 'browser' + ): + print(f'##### page: {entry}') + text = await asyncio.to_thread( + lambda: driver.find_element(By.TAG_NAME, 'body').text + ) + print(f'##### page text: {text[:300]!r}') + + await asyncio.sleep(0.5) + + async def _record_state_types(self) -> None: + """Puts what an API file would yield into the application. + + The reading of files is covered by `api_reader_tests` and + `api_watcher_tests`. What is left to show here is that the + page renders whatever the application holds, so this writes + that directly, and the test keeps no watcher, no observer + thread and no subprocess alive alongside a browser. + """ + context = self.rbt.create_external_context(name=self.id()) + await API.ref(API_ID).Update( + context, + state_types=[ + StateTypeInfo( + name='shop.v1.Shop', + file='api/shop/v1/shop.py', + fields=[FieldInfo(name='name', type='str')], + methods=[ + MethodInfo( + name='look', + kind='reader', + arguments=[FieldInfo(name='item', type='str')], + returns=[FieldInfo(name='found', type='bool')], + ), + ], + ), + ], + error='', + ) + + def _run(self, body): + driver = _driver() + try: + return body(driver) + finally: + print("##### Browser logs #####") + for entry in driver.get_log('browser'): + print(entry) + print("##### End of browser logs #####") + driver.quit() + + async def test_describes_what_the_api_files_declare(self) -> None: + # Nothing here is generated, built or serving: the page shows + # a state type because a file on disk declares one. + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + WebDriverWait(driver, 60).until( + expected_conditions.presence_of_element_located( + (By.ID, 'shop.v1.Shop') + ) + ) + return driver.page_source + + await self._record_state_types() + + page = await asyncio.to_thread(self._run, body) + + # A method, its kind, and its source file all come from the + # file rather than from anything the page knew in advance, + # spelled the way its author spelled them. + self.assertIn('look', page) + self.assertIn('reader', page) + self.assertIn('shop/v1/shop.py', page) + + # State types are grouped by their proto package, which is the + # directory the developer wrote them in. + self.assertIn('shop.v1', page) + + # The sidebar's two counts sit in the same column and count + # different things, so each says what it counts. The fixture + # declares one of each, which also covers the singular. + self.assertIn('1 state type', page) + self.assertIn('1 method', page) + + async def test_says_why_a_file_could_not_be_read(self) -> None: + # A half-written file is the normal case while someone is + # typing, so the page says what went wrong while keeping the + # shape it last read beside it. + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + WebDriverWait(driver, 60).until( + expected_conditions.presence_of_element_located( + (By.CLASS_NAME, 'error') + ) + ) + return driver.page_source + + context = self.rbt.create_external_context(name=self.id()) + await API.ref(API_ID).Update( + context, + state_types=[ + StateTypeInfo( + name='shop.v1.Shop', + file='api/shop/v1/shop.py', + fields=[FieldInfo(name='name', type='str')], + ), + ], + error='shop.py: SyntaxError: invalid syntax', + ) + + page = await asyncio.to_thread(self._run, body) + + self.assertIn('shop.py: SyntaxError: invalid syntax', page) + + # The error does not blank the page: what was last read is + # still there to work against. + self.assertIn('shop.v1.Shop', page) + + # The two labels the banner's one link shows, which are also the + # two things it does. + _TURN_OFF = "Don't reopen this dashboard on restart" + _TURN_ON = 'Open this dashboard on every restart' + + def _click_the_banner(self, driver, showing: str, becomes: str) -> None: + """Clicks the banner's link once it reads `showing`. + + Waits for `becomes` afterwards rather than returning as soon as + the click lands: the new label comes from the reactive read of + `Preferences`, so seeing it is how the test knows the choice + reached the application and came back. + """ + button = (By.CLASS_NAME, 'banner-link') + + WebDriverWait(driver, 60).until( + expected_conditions.text_to_be_present_in_element(button, showing) + ) + driver.find_element(*button).click() + WebDriverWait(driver, 60).until( + expected_conditions.text_to_be_present_in_element(button, becomes) + ) + + async def _suppress_open_on_restart(self) -> bool: + context = self.rbt.create_external_context(name=self.id()) + response = await Preferences.ref(PREFERENCES_ID).Get(context) + return response.suppress_open_on_restart + + async def test_the_banner_turns_reopening_off(self) -> None: + # What the banner writes is exactly what `rbt dev run` reads + # before deciding whether to open a dashboard, which is what + # `open_dashboard_tests` covers from the other side. + + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + self._click_the_banner( + driver, + showing=self._TURN_OFF, + becomes=self._TURN_ON, + ) + + await asyncio.to_thread(self._run, body) + + self.assertTrue(await self._suppress_open_on_restart()) + + async def test_the_banner_turns_reopening_back_on(self) -> None: + # A developer who clicked once is not stuck with it: the page + # they load next offers the choice the other way round. + + context = self.rbt.create_external_context(name=self.id()) + await Preferences.ref(PREFERENCES_ID).SetSuppressOpenOnRestart( + context, + suppress_open_on_restart=True, + ) + + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + self._click_the_banner( + driver, + showing=self._TURN_ON, + becomes=self._TURN_OFF, + ) + + await asyncio.to_thread(self._run, body) + + self.assertFalse(await self._suppress_open_on_restart()) + + # The two labels a state type's one button shows. + _EXPAND = 'Expand details' + _HIDE = 'Hide details' + + def _click_to_expand(self, driver, showing: str, becomes: str) -> None: + """Clicks a state type's button once it reads `showing`. + + Waits for `becomes` afterwards, which is how the test knows + the click was taken, since the label comes from the same state + the detail's height does. + """ + button = (By.CLASS_NAME, 'expand-button') + + WebDriverWait(driver, 60).until( + expected_conditions.text_to_be_present_in_element(button, showing) + ) + driver.find_element(*button).click() + WebDriverWait(driver, 60).until( + expected_conditions.text_to_be_present_in_element(button, becomes) + ) + + @staticmethod + def _detail_height(driver) -> float: + """How tall the first method's detail is drawn. + + Measured rather than asked of `is_displayed()`, because the + detail stays in the document whether or not its state type is + open: what closing does is collapse the grid row it sits in to + nothing, which is what makes the height animate at all. + """ + return driver.execute_script( + 'const detail = document.querySelector(".method-detail-inner");' + 'return detail === null' + ' ? -1' + ' : detail.getBoundingClientRect().height;' + ) + + def _wait_for_detail(self, driver, opened: bool) -> None: + """Waits out the animation, rather than sleeping its duration.""" + WebDriverWait( + driver, 60 + ).until(lambda driver: (self._detail_height(driver) > 0) == opened) + + async def _expanded_state_types(self) -> list[str]: + context = self.rbt.create_external_context(name=self.id()) + response = await Preferences.ref(PREFERENCES_ID).Get(context) + return list(response.expanded_state_types) + + async def test_expanding_a_state_type_opens_its_method_detail( + self + ) -> None: + await self._record_state_types() + + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + self._click_to_expand( + driver, + showing=self._EXPAND, + becomes=self._HIDE, + ) + self._wait_for_detail(driver, opened=True) + + # The height is transitioned rather than switched. Read + # off the property list rather than by sampling a height + # part-way through, which would be a race against the + # animation this is checking for. + self.assertIn( + 'grid-template-rows', + driver.execute_script( + 'const detail =' + ' document.querySelector(".method-detail");' + 'return getComputedStyle(detail).transitionProperty;' + ), + ) + + await asyncio.to_thread(self._run, body) + + # And the click reached the application, which is what makes + # it + # outlast the tab it was made in. + self.assertEqual(await self._expanded_state_types(), ['shop.v1.Shop']) + + async def test_a_state_type_expanded_earlier_is_open_on_load(self) -> None: + # The state a previous `rbt dev run` left behind, which is the + # whole reason the choice lives in the dashboard application. + await self._record_state_types() + + context = self.rbt.create_external_context(name=self.id()) + await Preferences.ref(PREFERENCES_ID).SetExpanded( + context, + state_type='shop.v1.Shop', + expanded=True, + ) + + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + WebDriverWait(driver, 60).until( + expected_conditions.text_to_be_present_in_element( + (By.CLASS_NAME, 'expand-button'), + self._HIDE, + ) + ) + self._wait_for_detail(driver, opened=True) + + await asyncio.to_thread(self._run, body) + + async def test_the_page_holds_presence(self) -> None: + # `rbt dev run` decides whether to open a dashboard by asking + # who is looking at one, so the page being counted while it is + # up and dropped once it is gone is what that decision rests + # on. + driver = await asyncio.to_thread(_driver) + try: + await asyncio.to_thread(driver.get, f'{self.url}{DASHBOARD_PATH}/') + + # Wait for the viewer to register, rather than assuming a + # page load is enough. + await self._wait_for_viewers( + lambda viewers: viewers != [], + driver=driver, + ) + finally: + await asyncio.to_thread(driver.quit) + + # With the browser gone the viewer must drain, which is what + # makes presence usable as a liveness signal at all. + await self._wait_for_viewers(lambda viewers: viewers == []) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/reboot/dashboard/open_dashboard_tests.py b/tests/reboot/dashboard/open_dashboard_tests.py new file mode 100644 index 00000000..cead0245 --- /dev/null +++ b/tests/reboot/dashboard/open_dashboard_tests.py @@ -0,0 +1,219 @@ +"""`rbt dev run` opens a dashboard when nobody is looking at one. + +The dashboard page subscribes to `Presence` for as long as it is open, +so the question the CLI asks is who is looking right now, which +reopens a dashboard the developer closed and never puts a second tab in +front of one they left up. It asks a second question first: whether the +developer clicked the dashboard's "Don't reopen this dashboard on +restart" banner, which is remembered in `Preferences`. + +The page's own subscription and its banner are exercised in +`dashboard_tests`; here the subscriber and the choice are made +directly, so these tests need no browser. +""" +import asyncio +import unittest +from rbt.dashboard.v1.dashboard_rbt import Preferences +from rbt.std.presence.subscriber.v1.subscriber_rbt import Subscriber +from rbt.std.presence.v1.presence_rbt import Presence +from rbt.v1alpha1.errors_pb2 import NotFound +from reboot.aio.tests import Reboot +from reboot.cli.commands.dev import _open_dashboard_once +from reboot.dashboard.constants import ( + DASHBOARD_PATH, + PREFERENCES_ID, + PRESENCE_ID, +) +from reboot.dashboard.main import application +from unittest.mock import patch + + +class OpenDashboardTest(unittest.IsolatedAsyncioTestCase): + + async def asyncSetUp(self) -> None: + self.rbt = Reboot() + await self.rbt.start() + await self.rbt.up(application(), local_envoy=True) + self.url = f'http://127.0.0.1:{self.rbt.envoy_port()}' + self.dashboard_url = f'{self.url}{DASHBOARD_PATH}/' + self._connections: list[asyncio.Task] = [] + + async def asyncTearDown(self) -> None: + for connection in self._connections: + connection.cancel() + await self.rbt.stop() + + async def _view(self, subscriber_id: str) -> None: + """Subscribes as a page would, and stays subscribed. + + `Connect` never returns, and `Toggle` has to land after it, so + the two run concurrently and `Toggle` is retried until the + connection it depends on exists, the same handshake + `reboot/std/react/presence` performs in the browser. + """ + context = self.rbt.create_external_context(name=self.id()) + subscriber = Subscriber.ref(subscriber_id) + nonce = subscriber_id + + await subscriber.idempotently().Create(context) + + self._connections.append( + asyncio.create_task(subscriber.Connect(context, nonce=nonce)) + ) + + attempt = 0 + while True: + try: + await subscriber.idempotently( + f'Attempt {attempt}', + ).Toggle(context, nonce=nonce) + break + except Subscriber.ToggleAborted as aborted: + if not isinstance(aborted.error, NotFound): + raise + attempt += 1 + + await Presence.ref(PRESENCE_ID).Subscribe( + context, + subscriber_id=subscriber_id, + ) + + async def _suppress_reopening(self, suppress: bool) -> None: + """Makes the choice the dashboard's banner makes.""" + context = self.rbt.create_external_context(name=self.id()) + await Preferences.ref(PREFERENCES_ID).SetSuppressOpenOnRestart( + context, + suppress_open_on_restart=suppress, + ) + + async def _viewers(self) -> list[str]: + context = self.rbt.create_external_context(name=self.id()) + try: + response = await Presence.ref(PRESENCE_ID).List(context) + return list(response.subscriber_ids) + except Presence.ListAborted: + return [] + + async def test_opens_when_nobody_is_looking(self) -> None: + self.assertEqual(await self._viewers(), []) + + with patch('webbrowser.open', return_value=True) as browser: + await _open_dashboard_once(dashboard_url=self.url, forced=False) + + # The browser gets the dashboard's path; `ExternalContext` only + # ever sees the origin, which is all it accepts. + browser.assert_called_once_with(self.dashboard_url) + + async def test_does_not_open_when_somebody_is_looking(self) -> None: + await self._view('a-tab-that-is-open') + self.assertEqual(await self._viewers(), ['a-tab-that-is-open']) + + with patch('webbrowser.open', return_value=True) as browser: + with patch('reboot.cli.common.terminal.info') as told: + await _open_dashboard_once( + dashboard_url=self.url, + forced=False, + ) + + browser.assert_not_called() + + # And it must say so: the tab being counted may be behind + # another window, so a run that opens nothing and explains + # nothing is indistinguishable from a broken one. + told.assert_called_once() + self.assertIn('--open-dashboard', told.call_args.args[0]) + self.assertIn(self.dashboard_url, told.call_args.args[0]) + + async def test_opens_again_once_the_last_viewer_has_gone(self) -> None: + await self._view('a-tab-that-closes') + + for connection in self._connections: + connection.cancel() + self._connections = [] + + # Cancelling `Connect` is the only signal presence has, and it + # reaches the subscriber list by way of `WaitForDisconnect` + # untoggling and `Watch` then dropping the subscriber, so wait + # for the list rather than assuming the cancellation was + # enough. + while await self._viewers() != []: + await asyncio.sleep(0.1) + + with patch('webbrowser.open', return_value=True) as browser: + await _open_dashboard_once(dashboard_url=self.url, forced=False) + + browser.assert_called_once_with(self.dashboard_url) + + async def test_forcing_opens_even_though_somebody_is_looking(self) -> None: + await self._view('a-tab-that-is-open') + + with patch('webbrowser.open', return_value=True) as browser: + await _open_dashboard_once(dashboard_url=self.url, forced=True) + + browser.assert_called_once_with(self.dashboard_url) + + async def test_does_not_open_when_the_developer_asked_it_not_to( + self + ) -> None: + # Nobody is looking at a dashboard, so the only thing keeping + # one from opening is the choice the banner recorded. + await self._suppress_reopening(True) + + with patch('webbrowser.open', return_value=True) as browser: + with patch('reboot.cli.common.terminal.info') as told: + await _open_dashboard_once( + dashboard_url=self.url, + forced=False, + ) + + browser.assert_not_called() + + # And it must say how to get one anyway, since a choice made + # in an earlier `rbt dev run` is not something the developer + # is looking at now. + told.assert_called_once() + self.assertIn('--open-dashboard', told.call_args.args[0]) + self.assertIn(self.dashboard_url, told.call_args.args[0]) + + async def test_forcing_opens_even_though_the_developer_asked_it_not_to( + self + ) -> None: + await self._suppress_reopening(True) + + with patch('webbrowser.open', return_value=True) as browser: + await _open_dashboard_once(dashboard_url=self.url, forced=True) + + browser.assert_called_once_with(self.dashboard_url) + + async def test_opens_again_once_the_developer_has_asked_for_it_back( + self + ) -> None: + # The second banner undoes the first, so a developer who + # clicked once is not stuck with it. + await self._suppress_reopening(True) + await self._suppress_reopening(False) + + with patch('webbrowser.open', return_value=True) as browser: + await _open_dashboard_once(dashboard_url=self.url, forced=False) + + browser.assert_called_once_with(self.dashboard_url) + + async def test_says_where_the_dashboard_is_when_none_could_be_opened( + self + ) -> None: + # `webbrowser.open` returns `False` rather than raising when + # there is nothing to open, which is the headless case. Nothing + # was shown, so the developer is told the address instead. + with patch('webbrowser.open', return_value=False): + with patch('reboot.cli.common.terminal.warn') as warned: + await _open_dashboard_once( + dashboard_url=self.url, + forced=False, + ) + + warned.assert_called_once() + self.assertIn(self.dashboard_url, warned.call_args.args[0]) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/reboot/dashboard/preferences_tests.py b/tests/reboot/dashboard/preferences_tests.py new file mode 100644 index 00000000..48be12fc --- /dev/null +++ b/tests/reboot/dashboard/preferences_tests.py @@ -0,0 +1,166 @@ +"""`Preferences` has a value before anybody has chosen one. + +The dashboard's banner renders from a reactive read of `Preferences`, +and a reader aborts with `StateNotConstructed` until something has +written, so the dashboard application writes the defaults at +startup. That write must not undo a choice the developer already +made, because it runs on every start of `rbt dashboard`, which is +exactly when a click from an earlier run has to survive. + +The banner that does the clicking is exercised in `dashboard_tests`, +and what `rbt dev run` does with the answer in `open_dashboard_tests`. +""" +import unittest +import uuid +from rbt.dashboard.v1.dashboard_rbt import Preferences +from reboot.aio.external import InitializeContext +from reboot.aio.tests import Reboot +from reboot.dashboard.constants import PREFERENCES_ID +from reboot.dashboard.main import application, initialize + + +class PreferencesTest(unittest.IsolatedAsyncioTestCase): + + async def asyncSetUp(self) -> None: + self.rbt = Reboot() + await self.rbt.start() + await self.rbt.up(application(), local_envoy=True) + self.url = f'http://127.0.0.1:{self.rbt.envoy_port()}' + + async def asyncTearDown(self) -> None: + await self.rbt.stop() + + def _initialize_context(self) -> InitializeContext: + """A restart's context, seeded as + `Reboot.create_initialize_context` seeds it.""" + return InitializeContext( + name=self.id(), + url=self.url, + idempotency_seed=uuid.uuid5( + uuid.NAMESPACE_DNS, 'anonymous.rbt.dev' + ), + ) + + async def _get(self) -> bool: + context = self.rbt.create_external_context(name=self.id()) + response = await Preferences.ref(PREFERENCES_ID).Get(context) + return response.suppress_open_on_restart + + async def _set_suppress(self, suppress: bool) -> None: + """Makes the choice the dashboard's banner makes.""" + context = self.rbt.create_external_context(name=self.id()) + await Preferences.ref(PREFERENCES_ID).SetSuppressOpenOnRestart( + context, + suppress_open_on_restart=suppress, + ) + + async def _expanded(self) -> list[str]: + context = self.rbt.create_external_context(name=self.id()) + response = await Preferences.ref(PREFERENCES_ID).Get(context) + return list(response.expanded_state_types) + + async def _set_expanded(self, state_type: str, expanded: bool) -> None: + """Makes the choice a state type's `Expand details` makes.""" + context = self.rbt.create_external_context(name=self.id()) + await Preferences.ref(PREFERENCES_ID).SetExpanded( + context, + state_type=state_type, + expanded=expanded, + ) + + async def test_starting_writes_a_default_that_can_be_read(self) -> None: + # The application's `initialize` constructed `Preferences` + # when it came up; a reader would otherwise abort with + # `StateNotConstructed`, and a page that loaded first would + # have nothing to render its banner from. + # + # False, so that somebody who has never clicked the banner gets + # a dashboard opened for them. + self.assertFalse(await self._get()) + + async def test_constructing_leaves_a_choice_already_made_alone( + self + ) -> None: + await self._set_suppress(True) + + await initialize(self._initialize_context()) + + self.assertTrue(await self._get()) + + async def test_constructing_twice_leaves_a_later_choice_alone( + self + ) -> None: + # The restart case: the dashboard constructs on every + # `rbt dashboard`, and the click it must not undo was made + # after the first of those. + await initialize(self._initialize_context()) + await self._set_suppress(True) + + await initialize(self._initialize_context()) + + self.assertTrue(await self._get()) + + async def test_collapsing_removes_the_state_type(self) -> None: + await self._set_expanded('bank.v1.Account', True) + await self._set_expanded('bank.v1.Bank', True) + + await self._set_expanded('bank.v1.Account', False) + + self.assertEqual(await self._expanded(), ['bank.v1.Bank']) + + async def test_expanding_twice_records_the_state_type_once(self) -> None: + # Two tabs can each send the same click, and a page that + # reconnects can send one it already sent. + await self._set_expanded('bank.v1.Account', True) + await self._set_expanded('bank.v1.Account', True) + + self.assertEqual(await self._expanded(), ['bank.v1.Account']) + + async def test_collapsing_what_was_never_expanded_is_no_error( + self + ) -> None: + await self._set_expanded('bank.v1.Account', False) + + self.assertEqual(await self._expanded(), []) + + async def test_the_order_clicked_in_does_not_change_what_is_stored( + self + ) -> None: + await self._set_expanded('bank.v1.Customer', True) + await self._set_expanded('bank.v1.Account', True) + + # Sorted, so that the reactive read does not push a change to + # every open page when the only difference is the order two + # clicks happened to arrive in. + self.assertEqual( + await self._expanded(), + ['bank.v1.Account', 'bank.v1.Customer'], + ) + + async def test_expanding_leaves_the_reopening_choice_alone(self) -> None: + # The reason there are two writers rather than one that takes + # both fields: a page that expands a state type must not write + # back a stale answer to a question it was not asked. + await self._set_suppress(True) + + await self._set_expanded('bank.v1.Account', True) + + self.assertTrue(await self._get()) + + async def test_the_reopening_choice_leaves_expansions_alone(self) -> None: + await self._set_expanded('bank.v1.Account', True) + + await self._set_suppress(True) + + self.assertEqual(await self._expanded(), ['bank.v1.Account']) + + async def test_constructing_leaves_expansions_alone(self) -> None: + await self._set_expanded('bank.v1.Account', True) + + await initialize(self._initialize_context()) + + self.assertEqual(await self._expanded(), ['bank.v1.Account']) + + +if __name__ == '__main__': + unittest.main() From 62dbe40f94825a31248b229e4fd1ba4316ca0716 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Wed, 12 Aug 2026 04:50:45 +0000 Subject: [PATCH 04/14] Allow a state type to have a description A method can say what it does, but a state type is the sum of its state and its methods, and its name alone does not say what it is for. `Type` now takes a description, which the dashboard shows beside the state type's name and file. Co-Authored-By: Claude Opus 5 (1M context) --- rbt/dashboard/v1/dashboard.proto | 3 + rbt/v1alpha1/inspect/inspect.proto | 51 -------- rbt/v1alpha1/options.proto | 6 + reboot/api.py | 3 + reboot/dashboard/frontend/dashboard.css | 26 ++++ reboot/dashboard/frontend/src/main.tsx | 35 ++++- reboot/inspect/BUILD.bazel | 13 -- reboot/inspect/describe_state_type.py | 141 --------------------- reboot/pydantic_schema_to_proto.py | 16 ++- tests/reboot/dashboard/api/shop/v1/shop.py | 8 +- tests/reboot/dashboard/api_reader_tests.py | 5 + 11 files changed, 97 insertions(+), 210 deletions(-) delete mode 100644 reboot/inspect/describe_state_type.py diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto index 38368499..3c5a8f01 100644 --- a/rbt/dashboard/v1/dashboard.proto +++ b/rbt/dashboard/v1/dashboard.proto @@ -33,6 +33,9 @@ message StateTypeInfo { string file = 2; repeated FieldInfo fields = 3; repeated MethodInfo methods = 4; + + // What the state type does, in the author's own words. + optional string description = 5; } //////////////////////////////////////////////////////////////////////// diff --git a/rbt/v1alpha1/inspect/inspect.proto b/rbt/v1alpha1/inspect/inspect.proto index 478a0e92..5031d26c 100644 --- a/rbt/v1alpha1/inspect/inspect.proto +++ b/rbt/v1alpha1/inspect/inspect.proto @@ -66,57 +66,6 @@ message GetStateResponse { //////////////////////////////////////////////////////////////////////// -// A field of a state, or an argument of a method. -message FieldInfo { - string name = 1; - - // Rendered for a reader, e.g. "str", "float", "AccountState". - string type = 2; -} - -message MethodInfo { - string name = 1; - - // One of "reader", "writer", "transaction" or "workflow". - string kind = 2; - - // The request message's fields, flattened. A method taking no - // request has none. - repeated FieldInfo arguments = 3; - - // The response's fields; empty when the method returns - // nothing. - repeated FieldInfo returns = 4; - - // Names of the error types the method declares it may raise. - repeated string errors = 5; - - // The description its author wrote, when there is one. - optional string description = 6; - - // Whether this method is a factory, constructing the state - // rather than requiring it to already exist. - bool factory = 7; - - // Whether the method is exposed as an MCP tool or resource. - bool mcp = 8; -} - -message StateTypeInfo { - // Fully qualified, e.g. "bank.v1.Account". - string name = 1; - - // The file the developer declared it in, e.g. - // "bank/v1/account.py". - string file = 2; - - repeated FieldInfo fields = 3; - - repeated MethodInfo methods = 4; -} - -//////////////////////////////////////////////////////////////////////// - service Inspect { // The list of state types in an application is static, however, we // make this a streaming RPC so that the client can hear when it diff --git a/rbt/v1alpha1/options.proto b/rbt/v1alpha1/options.proto index 9c8227ca..c6c1b261 100644 --- a/rbt/v1alpha1/options.proto +++ b/rbt/v1alpha1/options.proto @@ -163,6 +163,12 @@ message StateOptions { // `RBT_VALIDATE_TRUSTED_EFFECTS` is set. See // https://github.com/reboot-dev/mono/issues/4499. bool trusted_effects = 4; + + // What the state type does, in the author's own words. Worth + // writing about the parts a reader cannot derive, such as what it + // is the consistency boundary for, rather than restating the + // methods listed beside it. + optional string description = 5; } extend google.protobuf.MessageOptions { diff --git a/reboot/api.py b/reboot/api.py index 40c6d01e..0eda580b 100644 --- a/reboot/api.py +++ b/reboot/api.py @@ -1027,12 +1027,14 @@ class Type(pydantic.BaseModel): state: typing.Type[Model] methods: Methods + description: Optional[str] = None def __init__( self, *, state: typing.Type[Model], methods: Methods, + description: Optional[str] = None, ): def validate_all_fields_are_reboot_base_classes( @@ -1187,6 +1189,7 @@ def validate_all_fields_are_reboot_base_classes( super().__init__( state=state, methods=methods, + description=description, ) diff --git a/reboot/dashboard/frontend/dashboard.css b/reboot/dashboard/frontend/dashboard.css index f6dfb208..99ce79fa 100644 --- a/reboot/dashboard/frontend/dashboard.css +++ b/reboot/dashboard/frontend/dashboard.css @@ -252,6 +252,32 @@ header h1 { margin: 6px 0 4px; } +/* What the author says the state type is, under its name and + file. Wider measure and no padding of its own: it belongs to + the section's heading rather than to a card, the way + `.method-description` belongs to a method. */ +.state-type-description { + max-width: 62ch; + margin: 12px 0 0; + font-size: 14px; + line-height: 1.55; + color: hsl(var(--prose)); + text-wrap: pretty; +} + +/* Code inside a description, written as `backticks` by its author. + `0.9em` rather than a pixel size, so it follows whichever + description it sits in. */ +.state-type-description code, +.method-description code { + font-family: ui-monospace, Menlo, monospace; + font-size: 0.9em; + background: hsl(var(--surface-sunken)); + border: 1px solid hsl(var(--border-soft)); + border-radius: 4px; + padding: 0 4px; +} + .file { font-family: ui-monospace, Menlo, monospace; font-size: 11.5px; diff --git a/reboot/dashboard/frontend/src/main.tsx b/reboot/dashboard/frontend/src/main.tsx index b6fa0ecf..5c8a883f 100644 --- a/reboot/dashboard/frontend/src/main.tsx +++ b/reboot/dashboard/frontend/src/main.tsx @@ -75,6 +75,30 @@ const Kind: FC<{ kind: string }> = ({ kind }) => ( /> ); +// A description, with the spans its author wrote in `backticks` +// rendered as code rather than shown with their backticks. An +// unpaired backtick is kept as text, since it opens nothing. +const Description: FC<{ className: string; text: string }> = ({ + className, + text, +}) => { + const parts = text.split("`"); + return ( +

+ {parts.map((part, index) => { + // `split` alternates text and code, so odd indexes are code, + // except a last part at an odd index, whose backtick was + // never closed. + const unclosed = index === parts.length - 1 && parts.length % 2 === 0; + if (index % 2 === 1 && !unclosed) { + return {part}; + } + return {unclosed ? "`" + part : part}; + })} +

+ ); +}; + // A state type's namespace is its proto package: `bank.v1.Account` // lives in `bank.v1`, which is the developer's `api/bank/v1/`. const namespaceOf = (name: string): string => @@ -171,7 +195,10 @@ const Method: FC<{ method: MethodInfo }> = ({ method }) => {
{method.description !== undefined && ( -

{method.description}

+ )}
@@ -295,6 +322,12 @@ const StateType: FC<{
{stateType.file}
+ {stateType.description !== undefined && ( + + )}
state
{stateType.fields.length === 0 ? ( diff --git a/reboot/inspect/BUILD.bazel b/reboot/inspect/BUILD.bazel index 5083da6d..007e4e00 100644 --- a/reboot/inspect/BUILD.bazel +++ b/reboot/inspect/BUILD.bazel @@ -49,19 +49,6 @@ esbuild( platform = "browser", ) -py_library( - name = "describe_state_type_py", - srcs = ["describe_state_type.py"], - srcs_version = "PY3", - visibility = ["//visibility:public"], - deps = [ - "//log:log_py", - "//rbt/v1alpha1:options_py_proto", - "//rbt/v1alpha1/inspect:inspect_py_proto", - "//reboot/aio:types_py", - ], -) - py_library( name = "servicer_py", srcs = ["servicer.py"], diff --git a/reboot/inspect/describe_state_type.py b/reboot/inspect/describe_state_type.py deleted file mode 100644 index 62bbe43e..00000000 --- a/reboot/inspect/describe_state_type.py +++ /dev/null @@ -1,141 +0,0 @@ -"""Describes Reboot state types from protobuf descriptors. - -Everything comes from the descriptor pool it is handed: a state type's -fields, and the methods of the services that supply them. Field and -method types are rendered as the Python names a person reads on a -page, such as `int` and `list[str]`, rather than as their protobuf -spelling. -""" -from google.protobuf import descriptor_pool -from google.protobuf.descriptor import FieldDescriptor -from log.log import get_logger -from rbt.v1alpha1 import options_pb2 -from rbt.v1alpha1.inspect.inspect_pb2 import ( - FieldInfo, - MethodInfo, - StateTypeInfo, -) -from reboot.aio.types import StateTypeName -from typing import Iterable, Optional - -logger = get_logger(__name__) - -_TYPE_NAMES = { - FieldDescriptor.TYPE_DOUBLE: 'float', - FieldDescriptor.TYPE_FLOAT: 'float', - FieldDescriptor.TYPE_INT64: 'int', - FieldDescriptor.TYPE_UINT64: 'int', - FieldDescriptor.TYPE_INT32: 'int', - FieldDescriptor.TYPE_FIXED64: 'int', - FieldDescriptor.TYPE_FIXED32: 'int', - FieldDescriptor.TYPE_BOOL: 'bool', - FieldDescriptor.TYPE_STRING: 'str', - FieldDescriptor.TYPE_BYTES: 'bytes', - FieldDescriptor.TYPE_UINT32: 'int', - FieldDescriptor.TYPE_SFIXED32: 'int', - FieldDescriptor.TYPE_SFIXED64: 'int', - FieldDescriptor.TYPE_SINT32: 'int', - FieldDescriptor.TYPE_SINT64: 'int', -} - - -def _type_name(field) -> str: - """How to render `field`'s type.""" - if field.type in ( - FieldDescriptor.TYPE_MESSAGE, FieldDescriptor.TYPE_GROUP - ): - name = field.message_type.name - elif field.type == FieldDescriptor.TYPE_ENUM: - name = field.enum_type.name - else: - name = _TYPE_NAMES.get(field.type, 'unknown') - - if field.label == FieldDescriptor.LABEL_REPEATED: - return f'list[{name}]' - return name - - -def _fields_of(message) -> list[FieldInfo]: - return [ - FieldInfo(name=field.name, type=_type_name(field)) - for field in message.fields - ] - - -def _describe_method(method) -> MethodInfo: - options = method.GetOptions().Extensions[options_pb2.method] - kind = options.WhichOneof('kind') or '' - - info = MethodInfo( - name=method.name, - kind=kind, - arguments=_fields_of(method.input_type), - errors=list(options.errors), - mcp=options.HasField('mcp'), - ) - - # An empty response means the method returns nothing; saying - # "Empty" would be an implementation detail leaking out. The - # response's fields rather than its name, because a synthesized - # name such as `ShopRemainingResponse` says nothing the fields - # don't. - if method.output_type.full_name != 'google.protobuf.Empty': - info.returns.extend(_fields_of(method.output_type)) - - # An application that was created before `MethodOptions.description` - # will have the deprecated `mcp` description, which is permitted - # for backward compatibility. - if options.description: - info.description = options.description - elif options.HasField('mcp') and options.mcp.description: - info.description = options.mcp.description - - # Only writers and transactions can construct. - if kind in ('writer', 'transaction'): - info.factory = getattr(options, kind).HasField('constructor') - - return info - - -def describe_state_type( - pool: descriptor_pool.DescriptorPool, - state_type_name: StateTypeName, - service_names: Iterable[str], - file: str, -) -> Optional[StateTypeInfo]: - """Describes one state type, or `None` when its descriptors can't - be found, since a state type we can't describe shouldn't stop us - describing the rest. - - `pool` holds the state type's descriptors and those of the - services named in `service_names`, which supply its methods. - - `file` is the file the developer declared the state type in, which - is reported as it is. A Pydantic API is described from a `.proto` - synthesized from it, so the descriptors name a file that only - exists inside the build. - """ - try: - state = pool.FindMessageTypeByName(state_type_name) - except KeyError: - logger.warning( - f"No descriptor for state type '{state_type_name}'; " - "omitting it from the schema" - ) - return None - - info = StateTypeInfo( - name=state_type_name, - file=file, - fields=_fields_of(state), - ) - - for service_name in service_names: - try: - service = pool.FindServiceByName(service_name) - except KeyError: - continue - for method in service.methods: - info.methods.append(_describe_method(method)) - - return info diff --git a/reboot/pydantic_schema_to_proto.py b/reboot/pydantic_schema_to_proto.py index 2f774a1c..e42a6d32 100644 --- a/reboot/pydantic_schema_to_proto.py +++ b/reboot/pydantic_schema_to_proto.py @@ -173,6 +173,8 @@ async def generate( # Auto-construct enum value name for this state type, # or None for non-auto-constructed types. auto_construct: Optional[str] = None, + # What the state type does, in the author's own words. + description: Optional[str] = None, ): origin = get_origin(schema) args = get_args(schema) @@ -183,12 +185,19 @@ async def generate( await proto.write(f"message {name} {{\n") if state: - if uis or auto_construct: - # Generate state option with UIs and/or - # auto-construct annotation. Proto text + if uis or auto_construct or description is not None: + # Generate state option with UIs, a description + # and/or auto-construct annotation. Proto text # format uses repeated field names, not # array syntax. await proto.write(" option (rbt.v1alpha1.state) = {\n") + if description is not None: + # The description can contain `\` character, so we + # need to escape it for proto string literal. + await proto.write( + " description: " + f'"{_escape_string_for_proto(description)}"\n' + ) if auto_construct is not None: await proto.write( f" auto_construct: " @@ -811,6 +820,7 @@ async def generate_proto_file_from_api( uis=uis if uis else None, auto_construct=_PER_USER_ID if type_name == AUTO_CONSTRUCT_STATE_TYPE else None, + description=type_obj.description, ) await proto.write('\n') diff --git a/tests/reboot/dashboard/api/shop/v1/shop.py b/tests/reboot/dashboard/api/shop/v1/shop.py index f49b264d..efa1ca60 100644 --- a/tests/reboot/dashboard/api/shop/v1/shop.py +++ b/tests/reboot/dashboard/api/shop/v1/shop.py @@ -45,4 +45,10 @@ class OutOfStockError(Model): ), ) -api = API(Shop=Type(state=ShopState, methods=ShopMethods)) +api = API( + Shop=Type( + state=ShopState, + methods=ShopMethods, + description="A shop, and the stock it has to sell.", + ) +) diff --git a/tests/reboot/dashboard/api_reader_tests.py b/tests/reboot/dashboard/api_reader_tests.py index b5d3d23e..9cf31d72 100644 --- a/tests/reboot/dashboard/api_reader_tests.py +++ b/tests/reboot/dashboard/api_reader_tests.py @@ -41,6 +41,11 @@ async def test_describes_a_state_type_and_its_methods(self) -> None: os.path.join(API_DIRECTORY, 'shop/v1/shop.py'), ) + self.assertEqual( + shop['description'], + 'A shop, and the stock it has to sell.', + ) + self.assertEqual( [field['name'] for field in shop['fields']], ['name', 'open'], From 2d349ceaf08a5d9bf137cb3f4511c7916bac5257 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Thu, 13 Aug 2026 06:23:03 +0000 Subject: [PATCH 05/14] Give the manylinux builder a Python that ships `libpython` The manylinux images build every CPython with `--disable-shared` and delete even the static `libpython` archives. The `reboot-dev-reboot` genrule links `reboot_native.node` with `-lpython3.10`, a flag emitted by `python3.10-config --ldflags --embed`. That link has never been able to succeed inside these images. CI stayed green only while Bazel's remote cache served the genrule's outputs. The first cache miss made every platform fail deterministically. On x86_64 that miss came from a runner hardware swap: it changed the `lscpu` portion of `the_environment.txt`, and with it the whole cache scope. Point `python`/`python3` at a python-build-standalone CPython 3.10, which ships `libpython3.10.so`. It is the same build `reboot/nodejs/prepare_environment.sh` downloads. `pip`/`pip3` stay on the manylinux interpreter, whose layout `auditwheel` and the wheel builds expect. `python3` and `pip` therefore deliberately name different installations. Co-Authored-By: Claude Fable 5 --- Dockerfile | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index e587b446..d698e26c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -797,11 +797,25 @@ RUN dnf install -y clang gcc-c++ && dnf clean all \ && mkdir -p /usr/lib/llvm-${CLANG_VERSION}/lib \ && ln -sf /usr/lib/clang /usr/lib/llvm-${CLANG_VERSION}/lib/clang -# The `manylinux` image comes with multiple Python versions. We'll use -# 3.10 to match our main build environment. Set up symlinks so `python` -# and `python3` point to Python 3.10. -RUN ln -sf /opt/python/cp310-cp310/bin/python /usr/local/bin/python \ - && ln -sf /opt/python/cp310-cp310/bin/python /usr/local/bin/python3 \ +# The `manylinux` image comes with multiple Python versions, but all +# of them are built `--disable-shared` with no `libpython` installed, +# and embedding Python — which building `reboot_native.node` does — +# needs `-lpython3.10` at link time. Point `python` and `python3` at a +# python-build-standalone CPython 3.10 instead, which ships +# `libpython3.10.so`; it is the same build +# `reboot/nodejs/prepare_environment.sh` uses. `pip` and `pip3` stay +# on the manylinux 3.10, whose layout `auditwheel` and the wheel +# builds expect, so `python3` and `pip` deliberately name different +# installations. +RUN set -e; \ + if [ "${TARGETARCH}" = "amd64" ]; then ARCH=x86_64; else ARCH=aarch64; fi; \ + mkdir /tmp/python-build-standalone; \ + wget -qO- "https://github.com/indygreg/python-build-standalone/releases/download/20240814/cpython-3.10.14+20240814-${ARCH}-unknown-linux-gnu-install_only.tar.gz" \ + | tar -xzf - -C /tmp/python-build-standalone \ + && mv /tmp/python-build-standalone/python /opt/reboot-python \ + && rmdir /tmp/python-build-standalone \ + && ln -sf /opt/reboot-python/bin/python3 /usr/local/bin/python \ + && ln -sf /opt/reboot-python/bin/python3 /usr/local/bin/python3 \ && ln -sf /opt/python/cp310-cp310/bin/pip /usr/local/bin/pip \ && ln -sf /opt/python/cp310-cp310/bin/pip /usr/local/bin/pip3 From a1dce8ef67cf466df56c1dde7c4a8b1aca242453 Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:11:52 +0000 Subject: [PATCH 06/14] Find the Reboot calls a method makes, by reading its source The dashboard shows what each state type declares, but not how the state types relate: which of them a given method actually calls. That relationship only exists in the servicer implementations. This reads them. A call is a chain -- an entry (`Account.ref(id)`, a constructor, `self.ref()`), any number of modifiers (`.idempotently()`, `.schedule(when=)`, `.spawn()`, `.reactively()`, `.until(alias)`, ...), and the method at the end -- so the analysis walks that chain over the `ast`, tracking what each name is worth as it goes. Reading rather than importing, deliberately: importing would need the application's `sys.path` and its generated `_rbt` modules, which is to say a tree that builds, and the dashboard is meant to work before one does. Nothing here needs the generated code anyway, since `from bank.v1.account_rbt import Account` names `bank.v1.Account` outright. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- rbt/dashboard/v1/dashboard.proto | 49 ++ reboot/dashboard/BUILD.bazel | 10 + reboot/dashboard/call_analysis.py | 680 ++++++++++++++++++ tests/reboot/dashboard/BUILD.bazel | 9 + tests/reboot/dashboard/call_analysis_tests.py | 245 +++++++ 5 files changed, 993 insertions(+) create mode 100644 reboot/dashboard/call_analysis.py create mode 100644 tests/reboot/dashboard/call_analysis_tests.py diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto index 3c5a8f01..f0d95045 100644 --- a/rbt/dashboard/v1/dashboard.proto +++ b/rbt/dashboard/v1/dashboard.proto @@ -40,6 +40,55 @@ message StateTypeInfo { //////////////////////////////////////////////////////////////////////// +// One Reboot call that a method's implementation makes. +message Call { + // How the call is reached, which is the chain the developer wrote + // between the reference and the method. + enum How { + UNKNOWN = 0; + // A plain method call on a reference. + CALL = 1; + // A constructor, which both makes the state and hands back a + // reference to it. + CONSTRUCT = 2; + // Reached through `.schedule(when=...)`. + SCHEDULE = 3; + // Reached through `.spawn()`. + SPAWN = 4; + // Reached through `.reactively()`. + REACTIVELY = 5; + // Reached through `.until(alias)`. + UNTIL = 6; + // A workflow's `.read(context)`. + READ = 7; + // A workflow's `.write(context, ...)`. + WRITE = 8; + } + + // The state type called, spelled the way `StateTypeInfo` spells it, + // such as `bank.v1.Account`. + string state_type = 1; + + // The method called, spelled the way the call spells it, which is + // the way `MethodInfo` spells it too. Empty for `READ` and `WRITE`, + // which name no method. + string method = 2; + + How how = 3; +} + +// The calls one method's implementation makes. +message MethodCalls { + // The state type and method whose implementation this describes, + // spelled the way `StateTypeInfo` and `MethodInfo` spell them. + string state_type = 1; + string method = 2; + + repeated Call calls = 3; +} + +//////////////////////////////////////////////////////////////////////// + // What the dashboard application has read of the developer's API // files, so that a browser can read it without reaching the // application itself. diff --git a/reboot/dashboard/BUILD.bazel b/reboot/dashboard/BUILD.bazel index 4a7a1d0c..64186eec 100644 --- a/reboot/dashboard/BUILD.bazel +++ b/reboot/dashboard/BUILD.bazel @@ -33,6 +33,16 @@ py_library( ], ) +py_library( + name = "call_analysis_py", + srcs = ["call_analysis.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + "//rbt/dashboard/v1:dashboard_py_reboot", + ], +) + py_library( name = "servicers_py", srcs = ["servicers.py"], diff --git a/reboot/dashboard/call_analysis.py b/reboot/dashboard/call_analysis.py new file mode 100644 index 00000000..bddb3a23 --- /dev/null +++ b/reboot/dashboard/call_analysis.py @@ -0,0 +1,680 @@ +"""Finds the Reboot calls the developer's methods make. + +Reads their source rather than importing it. Importing would need the +application's `sys.path` and its generated `_rbt` modules, which is to +say a tree that builds; the dashboard is meant to work before one +does, and a half-written file is the normal case while someone is +typing. + +Nothing here needs the generated code anyway. A state type only +becomes reachable by importing a generated module, and +`from bank.v1.account_rbt import Account` names `bank.v1.Account` +outright -- the same spelling the API files produce. + +What the analysis cannot follow it records, as `Unanalyzed`, so that +what it does not know is visible rather than merely missing. +""" +import ast +import os +from dataclasses import dataclass +from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls +from typing import Optional, Union + +# The suffix a generated Python module carries. A name imported from +# one is a state type, and the package it came from qualifies it. +GENERATED_SUFFIX = '_rbt' + +# Chain methods that say when or how a call happens without changing +# what is called, so a call written through them still names a state +# type and a method. +_IDEMPOTENCY_MODIFIERS = frozenset( + [ + 'idempotently', + 'per_workflow', + 'per_iteration', + 'always', + ] +) + +# Chain methods that a call is still written through, but which say +# enough about the call to be worth reporting on their own. +_HOW_MODIFIERS: dict[str, 'Call.How.ValueType'] = { + 'reactively': Call.REACTIVELY, + 'until': Call.UNTIL, + 'schedule': Call.SCHEDULE, + 'spawn': Call.SPAWN, +} + +# Class methods that hand back a reference without calling anything. +_REFERENCE_ENTRIES = frozenset(['ref', 'forall']) + +# Reference methods that reach the state itself rather than one of its +# methods, and so name no method. +_STATE_TERMINALS: dict[str, 'Call.How.ValueType'] = { + 'read': Call.READ, + 'write': Call.WRITE, +} + +# What a servicer class defines that is not one of its state's +# methods. +_NOT_A_METHOD = frozenset(['authorizer']) + + +class _Context: + """The context a Reboot method is given, or anything bound from + it. Passing one to a function is what makes that function worth + following: it is how the function can make a call of its own.""" + + +_CONTEXT = _Context() + + +@dataclass(frozen=True) +class _StateClass: + """A generated state class, such as the `Account` bound by + `from bank.v1.account_rbt import Account`.""" + state_type: str + + +@dataclass(frozen=True) +class _Reference: + """A reference to a state, and how far along the chain between the + reference and a method the source has got.""" + state_type: str + how: 'Call.How.ValueType' + + +@dataclass(frozen=True) +class _Constructed: + """What a constructor hands back: a reference, and a response.""" + state_type: str + + +@dataclass(frozen=True) +class _Servicer: + """The `self` of a servicer method.""" + state_type: str + module: str + name: str + + +_Value = Union[_StateClass, _Reference, _Constructed, _Servicer, _Context, + None] + +_Function = Union[ast.FunctionDef, ast.AsyncFunctionDef] + + +def method_key(state_type: str, method: str) -> str: + """Names one method the way `Analysis`es are keyed.""" + return f'{state_type}.{method}' + + +@dataclass +class Module: + """One of the developer's source files, parsed.""" + + # Dotted, relative to the source directory, e.g. `bank_servicer`. + name: str + + # State classes this file imported, by the name it calls them: + # `Account` -> `bank.v1.Account`. + state_classes: dict[str, str] + + # Names this file imported from another file in the same tree, by + # the name it calls them: `helper` -> (`helpers`, `do_transfer`). + imports: dict[str, tuple[str, str]] + + # Whole modules this file imported, by the name it calls them: + # `helpers` -> `helpers`. + modules: dict[str, str] + + functions: dict[str, _Function] + classes: dict[str, ast.ClassDef] + + +def _state_type(module_name: str, name: str) -> Optional[str]: + """The state type a name imported from `module_name` refers to, + if `module_name` is a generated module. + + `bank.v1.account_rbt` and `Account` give `bank.v1.Account`: the + package qualifies the name, exactly as the directory of an API + file qualifies what it declares. + """ + if not module_name.endswith(GENERATED_SUFFIX): + return None + + if '.' not in module_name: + return name + + return f'{module_name.rsplit(".", 1)[0]}.{name}' + + +def parse(name: str, source: str) -> Module: + """Reads one source file. Raises `SyntaxError` on a file that is + half written, which is for the caller to report.""" + tree = ast.parse(source) + + module = Module( + name=name, + state_classes={}, + imports={}, + modules={}, + functions={}, + classes={}, + ) + + for statement in tree.body: + match statement: + case ast.Import(names=names): + for alias in names: + module.modules[alias.asname or alias.name] = alias.name + + # A relative import names no module of its own, and + # `from . import x` has nowhere to be read from here. + case ast.ImportFrom(module=str(imported), level=0, names=names): + for alias in names: + bound = alias.asname or alias.name + state_type = _state_type(imported, alias.name) + if state_type is not None: + module.state_classes[bound] = state_type + else: + module.imports[bound] = (imported, alias.name) + + case ast.FunctionDef(name=name) | ast.AsyncFunctionDef(name=name): + module.functions[name] = statement + + case ast.ClassDef(name=name): + module.classes[name] = statement + + return module + + +def _servicer_state_type( + class_definition: ast.ClassDef, + module: Module, +) -> Optional[str]: + """The state type a class services, if it services one. + + A servicer says so by what it inherits: `Account.Servicer`, or + `Account.singleton.Servicer` for a singleton. + """ + for base in class_definition.bases: + match base: + case ( + ast.Attribute(value=ast.Name(id=name), attr='Servicer') | + ast.Attribute( + value=ast. + Attribute(value=ast.Name(id=name), attr='singleton'), + attr='Servicer', + ) + ): + state_type = module.state_classes.get(name) + if state_type is not None: + return state_type + + return None + + +class _MethodAnalyzer: + """Finds what one method calls.""" + + def __init__(self, modules: dict[str, Module]): + self._modules = modules + self._calls: list[Call] = [] + + def analyze( + self, + module: Module, + servicer: _Servicer, + method: _Function, + ) -> MethodCalls: + environment = self._parameters(method, servicer) + + self._statements(method.body, module, environment) + + return MethodCalls( + state_type=servicer.state_type, + method=method.name, + calls=_unique(self._calls), + ) + + def _parameters( + self, + function: _Function, + servicer: Optional[_Servicer], + ) -> dict[str, _Value]: + """Binds a method's parameters. + + A Reboot method takes its context first, after `self` or `cls`. + """ + environment: dict[str, _Value] = {} + + names = [argument.arg for argument in function.args.args] + + if servicer is not None and len(names) > 0: + if names[0] in ('self', 'cls'): + environment[names[0]] = servicer + names = names[1:] + if len(names) > 0: + environment[names[0]] = _CONTEXT + + return environment + + ################################################################### + # Statements. + + def _statements( + self, + statements: list[ast.stmt], + module: Module, + environment: dict[str, _Value], + ) -> None: + for statement in statements: + self._statement(statement, module, environment) + + def _statement( + self, + statement: ast.stmt, + module: Module, + environment: dict[str, _Value], + ) -> None: + match statement: + case ast.Assign(targets=targets, value=value_node): + value = self._expression(value_node, module, environment) + for target in targets: + self._bind(target, value, module, environment) + + case ast.AnnAssign(target=target, value=ast.expr() as value_node): + value = self._expression(value_node, module, environment) + self._bind(target, value, module, environment) + + # `x += y` leaves `x` whatever it already was, so + # evaluating `y` is all there is to do. + case ast.AugAssign(value=value_node): + self._expression(value_node, module, environment) + + case ast.Return(value=ast.expr() as value_node): + self._expression(value_node, module, environment) + + # A function written inside a method can use the context it + # closes over, so it is read here, where what it closes + # over is known. + case ( + ast.FunctionDef(args=args, body=body) | + ast.AsyncFunctionDef(args=args, body=body) + ): + nested = dict(environment) + for parameter in args.args: + nested.pop(parameter.arg, None) + self._statements(body, module, nested) + + case ( + ast. + For(iter=iterated, target=target, body=body, orelse=orelse) | + ast.AsyncFor( + iter=iterated, target=target, body=body, orelse=orelse + ) + ): + self._expression(iterated, module, environment) + self._bind(target, None, module, environment) + self._statements(body, module, environment) + self._statements(orelse, module, environment) + + case ( + ast.With(items=items, body=body) | + ast.AsyncWith(items=items, body=body) + ): + for item in items: + value = self._expression( + item.context_expr, module, environment + ) + match item.optional_vars: + case ast.expr() as target: + self._bind(target, value, module, environment) + self._statements(body, module, environment) + + case ast.Try( + body=body, handlers=handlers, orelse=orelse, + finalbody=finalbody + ): + self._statements(body, module, environment) + for handler in handlers: + self._statements(handler.body, module, environment) + self._statements(orelse, module, environment) + self._statements(finalbody, module, environment) + + case ( + ast.If(test=test, body=body, orelse=orelse) | + ast.While(test=test, body=body, orelse=orelse) + ): + self._expression(test, module, environment) + self._statements(body, module, environment) + self._statements(orelse, module, environment) + + # A class written inside a method defines methods of its + # own, which are not this method's to read. + case ast.ClassDef(): + pass + + case _: + for child in ast.iter_child_nodes(statement): + match child: + case ast.expr(): + self._expression(child, module, environment) + + def _bind( + self, + target: ast.expr, + value: _Value, + module: Module, + environment: dict[str, _Value], + ) -> None: + match target: + case ast.Name(id=name): + environment[name] = value + + # A constructor hands back a reference and a response, so + # `account, _ = await Account.open(context, id)` binds a + # reference to the first name. + case ast.Tuple(elts=elements) | ast.List(elts=elements): + for index, element in enumerate(elements): + element_value: _Value = None + match value: + case _Constructed(state_type=state_type) if index == 0: + element_value = _Reference(state_type, Call.CALL) + self._bind(element, element_value, module, environment) + + ################################################################### + # Expressions. + + def _expression( + self, + node: ast.expr, + module: Module, + environment: dict[str, _Value], + ) -> _Value: + """What an expression is worth to the analysis, recording any + calls written inside it along the way.""" + match node: + case ast.Await(value=awaited): + return self._expression(awaited, module, environment) + + case ast.Name(id=name): + if name in environment: + return environment[name] + state_type = module.state_classes.get(name) + return None if state_type is None else _StateClass(state_type) + + case ast.Call(): + return self._call(node, module, environment) + + case ast.Attribute(value=value_node): + self._expression(value_node, module, environment) + return None + + case ast.Subscript(value=value_node, slice=index): + value = self._expression(value_node, module, environment) + self._expression(index, module, environment) + # The reference half of what a constructor handed back. + match value, index: + case _Constructed(state_type=state_type + ), ast.Constant(value=0): + return _Reference(state_type, Call.CALL) + return None + + case ( + ast.List(elts=elements) | ast.Tuple(elts=elements) | + ast.Set(elts=elements) + ): + for element in elements: + self._expression(element, module, environment) + return None + + case ast.Dict(keys=keys, values=values): + for key in keys: + # A `**rest` in a dict display names no key. + match key: + case ast.expr(): + self._expression(key, module, environment) + for value_node in values: + self._expression(value_node, module, environment) + return None + + case ( + ast.ListComp(generators=generators, elt=element) | + ast.SetComp(generators=generators, elt=element) | + ast.GeneratorExp(generators=generators, elt=element) + ): + self._comprehension(generators, [element], module, environment) + return None + + case ast.DictComp( + generators=generators, key=key, value=value_node + ): + self._comprehension( + generators, [key, value_node], module, environment + ) + return None + + case ast.Lambda(args=args, body=body): + nested = dict(environment) + for parameter in args.args: + nested.pop(parameter.arg, None) + self._expression(body, module, nested) + return None + + # Anything else is walked into rather than understood, so + # that a call written inside it is still found. + case _: + for child in ast.iter_child_nodes(node): + match child: + case ast.expr(): + self._expression(child, module, environment) + return None + + def _comprehension( + self, + generators: list[ast.comprehension], + elements: list[ast.expr], + module: Module, + environment: dict[str, _Value], + ) -> None: + nested = dict(environment) + for generator in generators: + self._expression(generator.iter, module, nested) + self._bind(generator.target, None, module, nested) + for condition in generator.ifs: + self._expression(condition, module, nested) + for element in elements: + self._expression(element, module, nested) + + ################################################################### + # Calls. + + def _call( + self, + node: ast.Call, + module: Module, + environment: dict[str, _Value], + ) -> _Value: + # Once, up front: evaluating an argument is what finds a call + # written inside it, and doing it twice would find it twice. + arguments = _Arguments( + positional=[ + self._expression(argument, module, environment) + for argument in node.args + ], + keyword={ + keyword.arg: + self._expression(keyword.value, module, environment) + for keyword in node.keywords + if keyword.arg is not None + }, + ) + + match node.func: + case ast.Attribute(value=value_node, attr=attribute): + receiver = self._expression(value_node, module, environment) + return self._receiver_call( + node, receiver, attribute, arguments, module, environment + ) + + case ast.Name(id=name): + return self._named_call( + node, name, arguments, module, environment + ) + + case _: + self._expression(node.func, module, environment) + return None + + def _receiver_call( + self, + node: ast.Call, + receiver: _Value, + attribute: str, + arguments: '_Arguments', + module: Module, + environment: dict[str, _Value], + ) -> _Value: + match receiver: + case _StateClass(state_type=state_type): + if attribute in _REFERENCE_ENTRIES: + return _Reference(state_type, Call.CALL) + + if attribute in _IDEMPOTENCY_MODIFIERS: + # `Account.per_workflow('open').open(context)`: + # still the class, still about to construct. + return receiver + + if not arguments.takes_context: + # A state class carries its request, response and + # error types too, and `Account.WithdrawAborted(...)` + # makes one of those rather than an account. A + # constructor is what takes the context first. + return None + + self._called(state_type, attribute, Call.CONSTRUCT) + return _Constructed(state_type) + + case _Reference(state_type=state_type, how=how): + if attribute in _IDEMPOTENCY_MODIFIERS: + return receiver + + modified = _HOW_MODIFIERS.get(attribute) + if modified is not None: + return _Reference(state_type, modified) + + terminal = _STATE_TERMINALS.get(attribute) + if terminal is not None: + self._called(state_type, '', terminal) + return None + + self._called(state_type, attribute, how) + return None + + case _Servicer(state_type=state_type): + if attribute == 'ref': + return _Reference(state_type, Call.CALL) + return None + + # The context's own API, such as `context.loop(...)`, is + # not a call to a state. + case _Context(): + return None + + case _: + return None + + def _named_call( + self, + node: ast.Call, + name: str, + arguments: '_Arguments', + module: Module, + environment: dict[str, _Value], + ) -> _Value: + # Calling a state class itself is not something the generated + # code offers; nothing to say about it. + return None + + def _called( + self, + state_type: str, + method: str, + how: 'Call.How.ValueType', + ) -> None: + self._calls.append(Call(state_type=state_type, method=method, how=how)) + + +@dataclass(frozen=True) +class _Arguments: + """What a call was given, already evaluated.""" + positional: list[_Value] + keyword: dict[str, _Value] + + @property + def takes_context(self) -> bool: + """Whether a context comes first, which is how every Reboot + method is called and how a constructor is told apart from the + request and error types a state class also carries.""" + return len(self.positional) > 0 and self.positional[0] is _CONTEXT + + +def _unique(messages: list) -> list: + """The same list without repeats, in the order they were found. + + A method that calls the same thing twice says nothing more than one + that calls it once. + """ + seen: set[bytes] = set() + unique = [] + for message in messages: + serialized = message.SerializeToString(deterministic=True) + if serialized not in seen: + seen.add(serialized) + unique.append(message) + return unique + + +def analyze(modules: dict[str, Module]) -> dict[str, MethodCalls]: + """Analyzes every servicer method in a tree of parsed files.""" + analyses: dict[str, MethodCalls] = {} + + for module in modules.values(): + for class_definition in module.classes.values(): + state_type = _servicer_state_type(class_definition, module) + if state_type is None: + continue + + servicer = _Servicer( + state_type=state_type, + module=module.name, + name=class_definition.name, + ) + + for statement in class_definition.body: + match statement: + case ( + ast.FunctionDef(name=name) | + ast.AsyncFunctionDef(name=name) + ) if ( + name not in _NOT_A_METHOD and not name.startswith('_') + ): + pass + case _: + continue + + key = method_key(state_type, name) + + analyses[key] = _MethodAnalyzer(modules).analyze( + module, servicer, statement + ) + + return analyses + + +def module_name(filename: str) -> str: + """The dotted name a source file goes by, relative to the source + directory it was found in.""" + return filename.rsplit('.py', 1)[0].replace(os.sep, '.') diff --git a/tests/reboot/dashboard/BUILD.bazel b/tests/reboot/dashboard/BUILD.bazel index c9956e23..e36f059e 100644 --- a/tests/reboot/dashboard/BUILD.bazel +++ b/tests/reboot/dashboard/BUILD.bazel @@ -22,6 +22,15 @@ py_test( ], ) +py_test( + name = "call_analysis_tests_py", + srcs = ["call_analysis_tests.py"], + main = "call_analysis_tests.py", + deps = [ + "//reboot/dashboard:call_analysis_py", + ], +) + py_test( name = "application_tests_py", srcs = [":application_tests.py"], diff --git a/tests/reboot/dashboard/call_analysis_tests.py b/tests/reboot/dashboard/call_analysis_tests.py new file mode 100644 index 00000000..ad7700ad --- /dev/null +++ b/tests/reboot/dashboard/call_analysis_tests.py @@ -0,0 +1,245 @@ +"""The dashboard works out what a method calls by reading it. + +Without importing it: `rbt generate` may not have run, no servicer has +to exist, and the file may be half written. Only the source. +""" +import unittest +from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls +from reboot.dashboard.call_analysis import analyze, parse + +# Every servicer below is written against this, so that the tests read +# as the developer's files do: a state class imported from a generated +# module, which is the only way one is reachable. +IMPORTS = ''' +from bank.v1.account_rbt import Account +from bank.v1.bank_rbt import Bank +''' + + +def _modules(**sources: str) -> dict: + return {name: parse(name, source) for name, source in sources.items()} + + +def _analyze(**sources: str) -> dict[str, MethodCalls]: + return analyze(_modules(**sources)) + + +def _calls(analyses: dict[str, MethodCalls], + key: str) -> list[tuple[str, str, str]]: + """What one method calls, as tuples that read like the source.""" + return [ + (call.state_type, call.method, Call.How.Name(call.how)) + for call in analyses[key].calls + ] + + +def _servicer(body: str, state: str = 'Account') -> str: + return IMPORTS + f''' + +class {state}Servicer({state}.Servicer): + + def authorizer(self): + return allow() + +{body} +''' + + +class CallAnalysisTest(unittest.TestCase): + + def test_a_chain_written_out_in_one_go(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def move(self, context, request): + await Account.ref(request.id).withdraw(context, amount=1) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.move'), + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + + def test_a_reference_held_in_a_local(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def transfer(self, context, request): + source = Account.ref(request.source) + target = Account.ref(request.target) + await source.withdraw(context, amount=request.amount) + await target.deposit(context, amount=request.amount) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.transfer'), + [ + ('bank.v1.Account', 'withdraw', 'CALL'), + ('bank.v1.Account', 'deposit', 'CALL'), + ], + ) + + def test_a_constructor_unpacked_into_a_reference(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def open(self, context, request): + account, _ = await Account.open(context, request.id) + await account.deposit(context, amount=request.initial) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.open'), + [ + ('bank.v1.Account', 'open', 'CONSTRUCT'), + ('bank.v1.Account', 'deposit', 'CALL'), + ], + ) + + def test_a_constructor_whose_result_is_thrown_away(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def sign_up(self, context, request): + await Account.create(context, request.id) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.sign_up'), + [('bank.v1.Account', 'create', 'CONSTRUCT')], + ) + + def test_a_state_class_also_carries_types_that_construct_nothing( + self + ) -> None: + """`Account.WithdrawAborted(...)` makes an error, not an + account. A constructor is what takes the context first.""" + analyses = _analyze( + servicer=_servicer( + ''' + async def balance(self, context): + if self.state.balance < 0: + raise Account.WithdrawAborted(Overdraft(amount=1)) + return Account.BalanceResponse(amount=self.state.balance) +''' + ) + ) + + self.assertEqual(_calls(analyses, 'bank.v1.Account.balance'), []) + + def test_the_servicers_own_state(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def interest(self, context): + await self.ref().schedule(when=timedelta(seconds=1)).interest(context) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.interest'), + [('bank.v1.Account', 'interest', 'SCHEDULE')], + ) + + def test_every_way_of_reaching_a_method(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def every(self, context, request): + await Account.ref(request.id).idempotently('once').deposit(context) + await Account.ref(request.id).per_iteration('each').withdraw(context) + await Account.ref(request.id).always().balance(context) + await Account.forall(request.ids).balance(context) + await Account.ref(request.id).spawn().interest(context) + await Account.ref(request.id).until('settled').balance(context) + async for update in Account.ref(request.id).reactively().balance( + context + ): + pass + await self.ref().read(context) + await self.ref().write(context, lambda state: state) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.every'), + [ + ('bank.v1.Account', 'deposit', 'CALL'), + ('bank.v1.Account', 'withdraw', 'CALL'), + ('bank.v1.Account', 'balance', 'CALL'), + ('bank.v1.Account', 'interest', 'SPAWN'), + ('bank.v1.Account', 'balance', 'UNTIL'), + ('bank.v1.Account', 'balance', 'REACTIVELY'), + ('bank.v1.Account', '', 'READ'), + ('bank.v1.Account', '', 'WRITE'), + ], + ) + + def test_a_call_made_of_every_id_at_once(self) -> None: + """`forall` says which states are called, not how the method is + reached, so it reads as the plain call it is.""" + analyses = _analyze( + servicer=_servicer( + ''' + async def balances(self, context, request): + await Account.forall(request.ids).balance(context) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.balances'), + [('bank.v1.Account', 'balance', 'CALL')], + ) + + def test_a_function_that_closes_over_the_context(self) -> None: + """It is given no context, so nothing is passed to follow; what + makes its calls findable is that it is read where what it + closes over is known.""" + analyses = _analyze( + servicer=_servicer( + ''' + async def balances(self, context, request): + + async def balance_of(id): + return await Account.ref(id).balance(context) + + return await asyncio.gather(*[balance_of(i) for i in request.ids]) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.balances'), + [('bank.v1.Account', 'balance', 'CALL')], + ) + + def test_the_same_call_written_twice_is_said_once(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def twice(self, context, request): + await Account.ref(request.id).deposit(context, amount=1) + await Account.ref(request.id).deposit(context, amount=2) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.twice'), + [('bank.v1.Account', 'deposit', 'CALL')], + ) + + +if __name__ == '__main__': + unittest.main() From 494908134e52ae9487a3c59fceff55317faafc6a Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:12:35 +0000 Subject: [PATCH 07/14] Follow the functions a method passes its context to A method rarely makes all of its calls itself. It hands its context to a helper, and that helper makes the call -- so a context reaching a function is exactly what makes that function worth reading, and a function that never gets one cannot call anything at all. Followed across files, into methods a servicer keeps on itself, and into functions written inside the method, which reach their context by closing over it rather than by being given it. A visited set stops a helper that calls itself. What cannot be followed is now said rather than silently dropped: a context reaching a function that cannot be read, a reference stored somewhere it cannot be tracked out of, or a method named only at run time. The discipline is that only what there is reason to believe is Reboot-related is recorded -- ordinary Python the analysis never claimed to follow is not, or the list would be every line in the file. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- rbt/dashboard/v1/dashboard.proto | 29 ++ reboot/dashboard/call_analysis.py | 253 +++++++++++++++++- tests/reboot/dashboard/call_analysis_tests.py | 188 ++++++++++++- 3 files changed, 461 insertions(+), 9 deletions(-) diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto index f0d95045..564d20b5 100644 --- a/rbt/dashboard/v1/dashboard.proto +++ b/rbt/dashboard/v1/dashboard.proto @@ -77,6 +77,34 @@ message Call { How how = 3; } +// Something in a method that the analysis could not follow, so that +// what it does not know is visible rather than merely missing. +// +// Only what there is reason to believe is Reboot-related is recorded. +// Ordinary Python the analysis never followed in the first place is +// not, or this would be a list of every line in the file. +message Unanalyzed { + enum Why { + UNKNOWN = 0; + // A context reached a function that could not be found or read, + // so whatever calls that function makes are unknown. + CONTEXT_PASSED_TO_UNKNOWN_FUNCTION = 1; + // A reference to a state type was stored or passed somewhere that + // could not be followed, so calls made through it later are + // unknown. + REFERENCE_ESCAPED = 2; + // A reference was called with a method name that the source does + // not spell out. + UNKNOWN_METHOD = 3; + } + + Why why = 1; + + // The expression this is about, as Python's `ast` writes it back + // out, which is how the developer finds it in their file. + string expression = 2; +} + // The calls one method's implementation makes. message MethodCalls { // The state type and method whose implementation this describes, @@ -85,6 +113,7 @@ message MethodCalls { string method = 2; repeated Call calls = 3; + repeated Unanalyzed unanalyzed = 4; } //////////////////////////////////////////////////////////////////////// diff --git a/reboot/dashboard/call_analysis.py b/reboot/dashboard/call_analysis.py index bddb3a23..39137db7 100644 --- a/reboot/dashboard/call_analysis.py +++ b/reboot/dashboard/call_analysis.py @@ -17,7 +17,7 @@ import ast import os from dataclasses import dataclass -from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls +from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls, Unanalyzed from typing import Optional, Union # The suffix a generated Python module carries. A name imported from @@ -221,6 +221,12 @@ class _MethodAnalyzer: def __init__(self, modules: dict[str, Module]): self._modules = modules self._calls: list[Call] = [] + self._unanalyzed: list[Unanalyzed] = [] + + # Functions being followed right now, so that a helper that + # calls itself, or two that call each other, stop rather than + # recurse forever. + self._following: set[str] = set() def analyze( self, @@ -236,6 +242,7 @@ def analyze( state_type=servicer.state_type, method=method.name, calls=_unique(self._calls), + unanalyzed=_unique(self._unanalyzed), ) def _parameters( @@ -294,7 +301,7 @@ def _statement( self._expression(value_node, module, environment) case ast.Return(value=ast.expr() as value_node): - self._expression(value_node, module, environment) + self._escapes(value_node, module, environment) # A function written inside a method can use the context it # closes over, so it is read here, where what it closes @@ -384,6 +391,13 @@ def _bind( element_value = _Reference(state_type, Call.CALL) self._bind(element, element_value, module, environment) + # Stored on an attribute or into a container, where calls + # made through it later cannot be followed. + case _: + match value: + case _Reference() | _Constructed(): + self._escaped(target) + ################################################################### # Expressions. @@ -427,7 +441,7 @@ def _expression( ast.Set(elts=elements) ): for element in elements: - self._expression(element, module, environment) + self._escapes(element, module, environment) return None case ast.Dict(keys=keys, values=values): @@ -435,9 +449,9 @@ def _expression( # A `**rest` in a dict display names no key. match key: case ast.expr(): - self._expression(key, module, environment) + self._escapes(key, module, environment) for value_node in values: - self._expression(value_node, module, environment) + self._escapes(value_node, module, environment) return None case ( @@ -486,7 +500,27 @@ def _comprehension( for condition in generator.ifs: self._expression(condition, module, nested) for element in elements: - self._expression(element, module, nested) + self._escapes(element, module, nested) + + def _escapes( + self, + node: ast.expr, + module: Module, + environment: dict[str, _Value], + ) -> None: + """Evaluates an expression in a place a reference cannot be + followed out of, recording it if one goes there.""" + match self._expression(node, module, environment): + case _Reference() | _Constructed(): + self._escaped(node) + + def _escaped(self, node: ast.expr) -> None: + self._unanalyzed.append( + Unanalyzed( + why=Unanalyzed.REFERENCE_ESCAPED, + expression=ast.unparse(node), + ) + ) ################################################################### # Calls. @@ -576,6 +610,9 @@ def _receiver_call( case _Servicer(state_type=state_type): if attribute == 'ref': return _Reference(state_type, Call.CALL) + self._follow_servicer_method( + node, receiver, attribute, arguments + ) return None # The context's own API, such as `context.loop(...)`, is @@ -584,6 +621,7 @@ def _receiver_call( return None case _: + self._follow_module_function(node, arguments, module) return None def _named_call( @@ -594,10 +632,160 @@ def _named_call( module: Module, environment: dict[str, _Value], ) -> _Value: - # Calling a state class itself is not something the generated - # code offers; nothing to say about it. + if name in module.state_classes: + # Calling a state class itself is not something the + # generated code offers; nothing to say about it. + return None + + match name, arguments.positional: + case 'getattr', [_Reference(), *_]: + self._unanalyzed.append( + Unanalyzed( + why=Unanalyzed.UNKNOWN_METHOD, + expression=ast.unparse(node), + ) + ) + return None + + self._follow_function(node, name, arguments, module) + + return None + + ################################################################### + # Following. + + def _follow_function( + self, + node: ast.Call, + name: str, + arguments: '_Arguments', + module: Module, + ) -> None: + """Follows a plain function call, when a context reaches it. + + A context is what lets a function make a call of its own, so + one that gets a context is worth reading; one that does not + cannot call anything. + """ + if not arguments.has_context: + return + + found = self._resolve_function(name, module) + + if found is None: + self._unfollowable(node) + return + + target_module, function = found + + self._follow(target_module, function, arguments) + + def _follow_module_function( + self, + node: ast.Call, + arguments: '_Arguments', + module: Module, + ) -> None: + """Follows a call written through a module, such as + `helpers.transfer(context, ...)`.""" + if not arguments.has_context: + return + + match node.func: + case ast.Attribute(value=ast.Name(id=name), + attr=attribute) if (name in module.modules): + other = self._modules.get(module.modules[name]) + if other is not None: + function = other.functions.get(attribute) + if function is not None: + self._follow(other, function, arguments) + return + + self._unfollowable(node) + + def _follow_servicer_method( + self, + node: ast.Call, + servicer: _Servicer, + attribute: str, + arguments: '_Arguments', + ) -> None: + """Follows a helper a servicer keeps on itself.""" + if not arguments.has_context: + return + + module = self._modules.get(servicer.module) + class_definition = ( + None if module is None else module.classes.get(servicer.name) + ) + + if module is not None and class_definition is not None: + for statement in class_definition.body: + match statement: + case ( + ast.FunctionDef(name=defined) | + ast.AsyncFunctionDef(name=defined) + ) if defined == attribute: + self._follow( + module, statement, arguments, servicer=servicer + ) + return + + self._unfollowable(node) + + def _unfollowable(self, node: ast.Call) -> None: + self._unanalyzed.append( + Unanalyzed( + why=Unanalyzed.CONTEXT_PASSED_TO_UNKNOWN_FUNCTION, + expression=ast.unparse(node), + ) + ) + + def _resolve_function( + self, + name: str, + module: Module, + ) -> Optional[tuple[Module, _Function]]: + function = module.functions.get(name) + if function is not None: + return (module, function) + + imported = module.imports.get(name) + if imported is not None: + other = self._modules.get(imported[0]) + if other is not None: + function = other.functions.get(imported[1]) + if function is not None: + return (other, function) + return None + def _follow( + self, + target_module: Module, + function: _Function, + arguments: '_Arguments', + servicer: Optional[_Servicer] = None, + ) -> None: + key = _function_key( + target_module, + function, + None if servicer is None else servicer.name, + ) + + if key in self._following: + return + + self._following.add(key) + try: + self._statements( + function.body, + target_module, + _followed_environment(function, arguments, servicer), + ) + finally: + self._following.discard(key) + def _called( self, state_type: str, @@ -613,6 +801,13 @@ class _Arguments: positional: list[_Value] keyword: dict[str, _Value] + @property + def has_context(self) -> bool: + return ( + any(value is _CONTEXT for value in self.positional) or + any(value is _CONTEXT for value in self.keyword.values()) + ) + @property def takes_context(self) -> bool: """Whether a context comes first, which is how every Reboot @@ -621,6 +816,48 @@ def takes_context(self) -> bool: return len(self.positional) > 0 and self.positional[0] is _CONTEXT +def _followed_environment( + function: _Function, + arguments: _Arguments, + servicer: Optional[_Servicer], +) -> dict[str, _Value]: + """Binds what the caller passed to what the callee calls it.""" + names = [argument.arg for argument in function.args.args] + + environment: dict[str, _Value] = {} + + if ( + servicer is not None and len(names) > 0 and + names[0] in ('self', 'cls') + ): + environment[names[0]] = servicer + names = names[1:] + + for index, value in enumerate(arguments.positional): + if index < len(names): + environment[names[index]] = value + + for name, value in arguments.keyword.items(): + if name in names: + environment[name] = value + + return environment + + +def _function_key( + module: Module, + function: _Function, + class_name: Optional[str] = None, +) -> str: + """Names one function, so that two of the same name -- a module's + and a class's -- are not taken for each other.""" + qualified = ( + function.name + if class_name is None else f'{class_name}.{function.name}' + ) + return f'{module.name}:{qualified}' + + def _unique(messages: list) -> list: """The same list without repeats, in the order they were found. diff --git a/tests/reboot/dashboard/call_analysis_tests.py b/tests/reboot/dashboard/call_analysis_tests.py index ad7700ad..07604a4f 100644 --- a/tests/reboot/dashboard/call_analysis_tests.py +++ b/tests/reboot/dashboard/call_analysis_tests.py @@ -4,7 +4,7 @@ to exist, and the file may be half written. Only the source. """ import unittest -from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls +from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls, Unanalyzed from reboot.dashboard.call_analysis import analyze, parse # Every servicer below is written against this, so that the tests read @@ -33,6 +33,14 @@ def _calls(analyses: dict[str, MethodCalls], ] +def _unanalyzed(analyses: dict[str, MethodCalls], + key: str) -> list[tuple[str, str]]: + return [ + (Unanalyzed.Why.Name(entry.why), entry.expression) + for entry in analyses[key].unanalyzed + ] + + def _servicer(body: str, state: str = 'Account') -> str: return IMPORTS + f''' @@ -134,6 +142,7 @@ async def balance(self, context): ) self.assertEqual(_calls(analyses, 'bank.v1.Account.balance'), []) + self.assertEqual(_unanalyzed(analyses, 'bank.v1.Account.balance'), []) def test_the_servicers_own_state(self) -> None: analyses = _analyze( @@ -202,6 +211,71 @@ async def balances(self, context, request): [('bank.v1.Account', 'balance', 'CALL')], ) + def test_a_helper_in_the_same_file(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def move(self, context, request): + await _transfer(context, request.amount) +''' + ''' + +async def _transfer(context, amount): + await Account.ref('a').withdraw(context, amount=amount) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.move'), + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + + def test_a_helper_in_another_file(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def move(self, context, request): + await transfer(context, request.amount) +''' + ).replace( + 'from bank.v1.bank_rbt import Bank', + 'from helpers import transfer', + ), + helpers=IMPORTS + ''' + +async def transfer(context, amount): + await Account.ref('a').withdraw(context, amount=amount) +''', + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.move'), + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + + def test_a_helper_reached_through_its_module(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def move(self, context, request): + await helpers.transfer(context, request.amount) +''' + ).replace( + 'from bank.v1.bank_rbt import Bank', + 'import helpers', + ), + helpers=IMPORTS + ''' + +async def transfer(context, amount): + await Account.ref('a').withdraw(context, amount=amount) +''', + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.move'), + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + def test_a_function_that_closes_over_the_context(self) -> None: """It is given no context, so nothing is passed to follow; what makes its calls findable is that it is read where what it @@ -224,6 +298,26 @@ async def balance_of(id): [('bank.v1.Account', 'balance', 'CALL')], ) + def test_a_helper_that_calls_itself(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def sweep(self, context, request): + await _sweep(context, request.id) +''' + ''' + +async def _sweep(context, id): + await Account.ref(id).withdraw(context, amount=1) + await _sweep(context, id) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.sweep'), + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + def test_the_same_call_written_twice_is_said_once(self) -> None: analyses = _analyze( servicer=_servicer( @@ -240,6 +334,98 @@ async def twice(self, context, request): [('bank.v1.Account', 'deposit', 'CALL')], ) + ################################################################### + # What it cannot follow. + + def test_a_context_reaching_a_function_it_cannot_read(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def move(self, context, request): + await elsewhere.transfer(context, request.amount) +''' + ) + ) + + self.assertEqual(_calls(analyses, 'bank.v1.Account.move'), []) + self.assertEqual( + _unanalyzed(analyses, 'bank.v1.Account.move'), + [ + ( + 'CONTEXT_PASSED_TO_UNKNOWN_FUNCTION', + 'elsewhere.transfer(context, request.amount)', + ), + ], + ) + + def test_a_reference_stored_where_it_cannot_be_followed(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def keep(self, context, request): + self.account = Account.ref(request.id) +''' + ) + ) + + self.assertEqual( + _unanalyzed(analyses, 'bank.v1.Account.keep'), + [('REFERENCE_ESCAPED', 'self.account')], + ) + + def test_a_reference_put_into_a_container(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def gather(self, context, request): + accounts = [Account.ref(id) for id in request.ids] +''' + ) + ) + + self.assertEqual( + _unanalyzed(analyses, 'bank.v1.Account.gather'), + [('REFERENCE_ESCAPED', 'Account.ref(id)')], + ) + + def test_a_method_the_source_does_not_spell(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def dynamic(self, context, request): + await getattr(Account.ref(request.id), request.method)(context) +''' + ) + ) + + self.assertEqual( + _unanalyzed(analyses, 'bank.v1.Account.dynamic'), + [ + ( + 'UNKNOWN_METHOD', + 'getattr(Account.ref(request.id), request.method)', + ), + ], + ) + + def test_ordinary_python_is_not_reported(self) -> None: + """Everything the analysis never claimed to follow would drown + out the little it genuinely could not.""" + analyses = _analyze( + servicer=_servicer( + ''' + async def ordinary(self, context, request): + logging.info('a balance was read') + total = sum(entry.amount for entry in self.state.entries) + self.state.balance = round(total, 2) + return Account.BalanceResponse(amount=self.state.balance) +''' + ) + ) + + self.assertEqual(_calls(analyses, 'bank.v1.Account.ordinary'), []) + self.assertEqual(_unanalyzed(analyses, 'bank.v1.Account.ordinary'), []) + if __name__ == '__main__': unittest.main() From 26f802ed20255f846ca72f93c883d44ab28fdb3d Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:13:03 +0000 Subject: [PATCH 08/14] Resolve a state class however the developer imported it Importing a state class from its own generated module is only the plainest of the ways an application reaches one, and until now it was the only way this recognised. Real applications also: - import the standard library's states from the `reboot.std.` module that wraps each one and re-exports it, rather than from the `rbt.std....._rbt` module the wrapper stands in for; - import a state class from another of their own files, which imported it first; - bind a whole generated module and reach the states through it, as `from rbt.thirdparty.mailgun.v1 import mailgun_rbt as mailgun` then `mailgun.Message`; - keep a reference behind a property, as `return SortedMap.ref(INDEX_ID)`, and call through `self`. All four now resolve, which is what turns a handful of "could not be followed" entries on real applications into the calls they always were. A property is also no longer mistaken for a method of the state. Docstrings are dropped when a file is parsed: the analysis reads what the code does, and a property whose body is one `return` is still one `return` with prose above it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- reboot/dashboard/call_analysis.py | 286 ++++++++++++++++-- tests/reboot/dashboard/call_analysis_tests.py | 83 +++++ 2 files changed, 342 insertions(+), 27 deletions(-) diff --git a/reboot/dashboard/call_analysis.py b/reboot/dashboard/call_analysis.py index 39137db7..68989c77 100644 --- a/reboot/dashboard/call_analysis.py +++ b/reboot/dashboard/call_analysis.py @@ -24,6 +24,15 @@ # one is a state type, and the package it came from qualifies it. GENERATED_SUFFIX = '_rbt' +# The standard library's states are declared under `rbt.std.` and +# generated into `..._rbt` like any other, but an application imports +# them from the `reboot.std.` module that wraps each one and re-exports +# it -- `from reboot.std.collections.v1.sorted_map import SortedMap`. +# The two paths run in step, so a name imported from the wrapper names +# the state its generated module would. +STANDARD_LIBRARY_MODULE = 'reboot.std.' +STANDARD_LIBRARY_PACKAGE = 'rbt.std.' + # Chain methods that say when or how a call happens without changing # what is called, so a call written through them still names a state # type and a method. @@ -59,6 +68,18 @@ # methods. _NOT_A_METHOD = frozenset(['authorizer']) +# Decorators that say the same: a Reboot method is a plain `def`, or a +# `classmethod` when it is a workflow. A property is how an +# application names a state it uses throughout, not a method of its +# own. +_NOT_A_METHOD_DECORATORS = frozenset( + [ + 'property', + 'cached_property', + 'staticmethod', + ] +) + class _Context: """The context a Reboot method is given, or anything bound from @@ -109,6 +130,27 @@ def method_key(state_type: str, method: str) -> str: return f'{state_type}.{method}' +def _docstring_stripped(body: list[ast.stmt]) -> list[ast.stmt]: + match body: + case [ast.Expr(value=ast.Constant(value=str())), *rest]: + return rest + case _: + return body + + +def _strip_docstrings(tree: ast.AST) -> None: + """Drops every docstring, so that what is left is what the code + does. Prose is no more a part of that than a comment is, and both + would otherwise make a method look changed when it is not.""" + for node in ast.walk(tree): + match node: + case ( + ast.Module() | ast.ClassDef() | ast.FunctionDef() | + ast.AsyncFunctionDef() + ): + node.body = _docstring_stripped(node.body) + + @dataclass class Module: """One of the developer's source files, parsed.""" @@ -139,14 +181,24 @@ def _state_type(module_name: str, name: str) -> Optional[str]: `bank.v1.account_rbt` and `Account` give `bank.v1.Account`: the package qualifies the name, exactly as the directory of an API file qualifies what it declares. - """ - if not module_name.endswith(GENERATED_SUFFIX): - return None + A standard-library wrapper qualifies it the same way, through the + `rbt.std.` package its module stands in for: + `reboot.std.collections.v1.sorted_map` and `SortedMap` give + `rbt.std.collections.v1.SortedMap`. + """ if '.' not in module_name: - return name + return name if module_name.endswith(GENERATED_SUFFIX) else None + + package = module_name.rsplit('.', 1)[0] - return f'{module_name.rsplit(".", 1)[0]}.{name}' + if module_name.startswith(STANDARD_LIBRARY_MODULE): + package = STANDARD_LIBRARY_PACKAGE + package[ + len(STANDARD_LIBRARY_MODULE):] + elif not module_name.endswith(GENERATED_SUFFIX): + return None + + return f'{package}.{name}' def parse(name: str, source: str) -> Module: @@ -154,6 +206,8 @@ def parse(name: str, source: str) -> Module: half written, which is for the caller to report.""" tree = ast.parse(source) + _strip_docstrings(tree) + module = Module( name=name, state_classes={}, @@ -175,8 +229,16 @@ def parse(name: str, source: str) -> Module: for alias in names: bound = alias.asname or alias.name state_type = _state_type(imported, alias.name) + within = f'{imported}.{alias.name}' if state_type is not None: module.state_classes[bound] = state_type + elif _is_generated(within): + # A generated module imported as a whole, as + # `from rbt.thirdparty.mailgun.v1 import + # mailgun_rbt as mailgun`, so that the states + # in it are reached through the name it was + # bound to. + module.modules[bound] = within else: module.imports[bound] = (imported, alias.name) @@ -189,7 +251,77 @@ def parse(name: str, source: str) -> Module: return module +def _is_generated(module_name: str) -> bool: + """Whether a module is one the states are reached through.""" + return ( + module_name.endswith(GENERATED_SUFFIX) or + module_name.startswith(STANDARD_LIBRARY_MODULE) + ) + + +def _module_named( + modules: dict[str, Module], + name: str, +) -> Optional[Module]: + """The parsed file an import names. + + A file is keyed by where it sits under the source directory, but is + imported by whatever `sys.path` makes it -- flatly as + `account_servicer` when the directory is itself on the path, and as + `backend.src.account_servicer` when a directory above it is. Trying + each suffix in turn reads both the same way. + """ + module = modules.get(name) + if module is not None: + return module + + parts = name.split('.') + + for index in range(1, len(parts)): + module = modules.get('.'.join(parts[index:])) + if module is not None: + return module + + return None + + +def _state_class( + modules: dict[str, Module], + module: Module, + name: str, +) -> Optional[str]: + """The state type a name stands for in one file. + + A file may import one from its generated module, or from another of + the developer's files that imported it first -- both name the same + state, so a re-export is followed until a generated module answers + or nothing does. + """ + seen: set[str] = set() + + while True: + state_type = module.state_classes.get(name) + if state_type is not None: + return state_type + + imported = module.imports.get(name) + if imported is None: + return None + + key = f'{module.name}:{name}' + if key in seen: + return None + seen.add(key) + + other = _module_named(modules, imported[0]) + if other is None: + return None + + module, name = other, imported[1] + + def _servicer_state_type( + modules: dict[str, Module], class_definition: ast.ClassDef, module: Module, ) -> Optional[str]: @@ -208,7 +340,7 @@ def _servicer_state_type( attr='Servicer', ) ): - state_type = module.state_classes.get(name) + state_type = _state_class(modules, module, name) if state_type is not None: return state_type @@ -416,14 +548,26 @@ def _expression( case ast.Name(id=name): if name in environment: return environment[name] - state_type = module.state_classes.get(name) + state_type = _state_class(self._modules, module, name) return None if state_type is None else _StateClass(state_type) case ast.Call(): return self._call(node, module, environment) - case ast.Attribute(value=value_node): - self._expression(value_node, module, environment) + case ast.Attribute(value=value_node, attr=attribute): + # A state reached through the module it was bound to, + # as `mailgun.Message`. + match value_node: + case ast.Name(id=name) if name in module.modules: + state_type = _state_type( + module.modules[name], attribute + ) + if state_type is not None: + return _StateClass(state_type) + + match self._expression(value_node, module, environment): + case _Servicer() as servicer: + return self._servicer_attribute(servicer, attribute) return None case ast.Subscript(value=value_node, slice=index): @@ -610,6 +754,10 @@ def _receiver_call( case _Servicer(state_type=state_type): if attribute == 'ref': return _Reference(state_type, Call.CALL) + if not arguments.has_context: + # Nothing it could call, but it may still hand back + # a reference, as `self._index()` does. + return self._servicer_attribute(receiver, attribute) self._follow_servicer_method( node, receiver, attribute, arguments ) @@ -632,7 +780,7 @@ def _named_call( module: Module, environment: dict[str, _Value], ) -> _Value: - if name in module.state_classes: + if _state_class(self._modules, module, name) is not None: # Calling a state class itself is not something the # generated code offers; nothing to say about it. return None @@ -694,7 +842,7 @@ def _follow_module_function( match node.func: case ast.Attribute(value=ast.Name(id=name), attr=attribute) if (name in module.modules): - other = self._modules.get(module.modules[name]) + other = _module_named(self._modules, module.modules[name]) if other is not None: function = other.functions.get(attribute) if function is not None: @@ -711,27 +859,92 @@ def _follow_servicer_method( arguments: '_Arguments', ) -> None: """Follows a helper a servicer keeps on itself.""" - if not arguments.has_context: + found = self._servicer_function(servicer, attribute) + + if found is None: + self._unfollowable(node) return + module, function = found + + self._follow(module, function, arguments, servicer=servicer) + + def _servicer_function( + self, + servicer: _Servicer, + name: str, + ) -> Optional[tuple[Module, _Function]]: + """A function a servicer class defines, and the file it is + written in.""" module = self._modules.get(servicer.module) + class_definition = ( None if module is None else module.classes.get(servicer.name) ) - if module is not None and class_definition is not None: - for statement in class_definition.body: - match statement: - case ( - ast.FunctionDef(name=defined) | - ast.AsyncFunctionDef(name=defined) - ) if defined == attribute: - self._follow( - module, statement, arguments, servicer=servicer - ) - return + if module is None or class_definition is None: + return None - self._unfollowable(node) + for statement in class_definition.body: + match statement: + case ( + ast.FunctionDef(name=defined) | + ast.AsyncFunctionDef(name=defined) + ) if defined == name: + return (module, statement) + + return None + + def _servicer_attribute( + self, + servicer: _Servicer, + name: str, + ) -> _Value: + """What a helper a servicer keeps on itself hands back. + + Naming a state an application uses throughout by keeping it + behind a property is the ordinary way to write it: + + @property + def _applications_index(self): + return SortedMap.ref(APPLICATIONS_INDEX_ID) + + so `self._applications_index.insert(context, ...)` is as much a + call as writing the reference out would have been. Only a body + that is a single `return` is read this way; a longer one is not + something one value can stand for, and is left unanalyzed + rather than guessed at. + """ + found = self._servicer_function(servicer, name) + + if found is None: + return None + + module, function = found + + match function.body: + case [ast.Return(value=ast.expr() as returned)]: + pass + case _: + return None + + key = _function_key(module, function, servicer.name) + + if key in self._following: + return None + + self._following.add(key) + try: + return self._expression( + returned, + module, + { + 'self': servicer, + 'cls': servicer + }, + ) + finally: + self._following.discard(key) def _unfollowable(self, node: ast.Call) -> None: self._unanalyzed.append( @@ -752,7 +965,7 @@ def _resolve_function( imported = module.imports.get(name) if imported is not None: - other = self._modules.get(imported[0]) + other = _module_named(self._modules, imported[0]) if other is not None: function = other.functions.get(imported[1]) if function is not None: @@ -844,6 +1057,21 @@ def _followed_environment( return environment +def _decorated(function: _Function, names: frozenset) -> bool: + """Whether a function carries any of `names` as a decorator, + however it was spelled: `property` or `functools.cached_property` + alike.""" + for decorator in function.decorator_list: + match decorator: + case ( + ast.Call(func=ast.Name(id=name)) | ast.Name(id=name) | + ast.Call(func=ast.Attribute(attr=name)) | + ast.Attribute(attr=name) + ) if name in names: + return True + return False + + def _function_key( module: Module, function: _Function, @@ -880,7 +1108,9 @@ def analyze(modules: dict[str, Module]) -> dict[str, MethodCalls]: for module in modules.values(): for class_definition in module.classes.values(): - state_type = _servicer_state_type(class_definition, module) + state_type = _servicer_state_type( + modules, class_definition, module + ) if state_type is None: continue @@ -896,7 +1126,9 @@ def analyze(modules: dict[str, Module]) -> dict[str, MethodCalls]: ast.FunctionDef(name=name) | ast.AsyncFunctionDef(name=name) ) if ( - name not in _NOT_A_METHOD and not name.startswith('_') + name not in _NOT_A_METHOD and + not name.startswith('_') and + not _decorated(statement, _NOT_A_METHOD_DECORATORS) ): pass case _: diff --git a/tests/reboot/dashboard/call_analysis_tests.py b/tests/reboot/dashboard/call_analysis_tests.py index 07604a4f..dc7ecfe3 100644 --- a/tests/reboot/dashboard/call_analysis_tests.py +++ b/tests/reboot/dashboard/call_analysis_tests.py @@ -211,6 +211,89 @@ async def balances(self, context, request): [('bank.v1.Account', 'balance', 'CALL')], ) + def test_a_reference_kept_behind_a_property(self) -> None: + """Naming a state an application uses throughout by keeping a + reference behind a property is the ordinary way to write it, so + a call through one is as much a call as writing the reference + out would have been.""" + analyses = _analyze( + servicer=_servicer( + ''' + @property + def index(self): + return Bank.ref('index') + + async def record(self, context, request): + await self.index.note(context, what=request.what) +''' + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.record'), + [('bank.v1.Bank', 'note', 'CALL')], + ) + + # And the property is not itself one of the state's methods. + self.assertNotIn('bank.v1.Account.index', analyses) + + def test_a_state_from_the_standard_library(self) -> None: + """Its states are declared under `rbt.std.` but imported from + the `reboot.std.` module that wraps each one.""" + analyses = _analyze( + servicer=_servicer( + ''' + async def record(self, context, request): + await SortedMap.ref('index').insert(context, entries={}) +''' + ).replace( + 'from bank.v1.bank_rbt import Bank', + 'from reboot.std.collections.v1.sorted_map import SortedMap', + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.record'), + [('rbt.std.collections.v1.SortedMap', 'insert', 'CALL')], + ) + + def test_a_state_class_another_file_imported_first(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def look(self, context, request): + await Depot.ref(request.id).stock(context) +''' + ).replace( + 'from bank.v1.bank_rbt import Bank', + 'from depot_servicer import Depot', + ), + depot_servicer='from bank.v1.depot_rbt import Depot\n', + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.look'), + [('bank.v1.Depot', 'stock', 'CALL')], + ) + + def test_a_generated_module_imported_whole(self) -> None: + analyses = _analyze( + servicer=_servicer( + ''' + async def notify(self, context, request): + await mail.Message.Send(context, request.id, recipient=request.to) +''' + ).replace( + 'from bank.v1.bank_rbt import Bank', + 'from bank.v1 import mail_rbt as mail', + ) + ) + + self.assertEqual( + _calls(analyses, 'bank.v1.Account.notify'), + [('bank.v1.Message', 'Send', 'CONSTRUCT')], + ) + def test_a_helper_in_the_same_file(self) -> None: analyses = _analyze( servicer=_servicer( From bf36fe228fcedb21ccfe7e6f9cd3feb49fe9fe5d Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:13:32 +0000 Subject: [PATCH 09/14] Analyze again only the methods whose code has changed Reading a whole tree on every keystroke costs what it costs; reading the one method somebody just edited need not. Each method is hashed by `ast.dump` with `include_attributes=False`, which leaves out line and column numbers, so reflowing an argument list or moving a method down a file gives back the same hash. Comments never reach the tree at all, and docstrings are already stripped when the file is parsed, so neither makes a method look changed when it is not. Hashing the method alone would go stale, because its answer depends on everything the analysis walked through to reach it. So what is recorded is the hash of the method *and* of every function followed, however many calls away: a change to a helper two files over invalidates the method that reaches it, and nothing else does. What produced a result is a hash of this module's own source, so an analysis that has since changed cannot have its old conclusions mistaken for what it would say now -- without anybody having to remember to raise a number. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- reboot/dashboard/call_analysis.py | 125 +++++++++++++- tests/reboot/dashboard/call_analysis_tests.py | 161 +++++++++++++++++- 2 files changed, 270 insertions(+), 16 deletions(-) diff --git a/reboot/dashboard/call_analysis.py b/reboot/dashboard/call_analysis.py index 68989c77..8310805c 100644 --- a/reboot/dashboard/call_analysis.py +++ b/reboot/dashboard/call_analysis.py @@ -15,11 +15,33 @@ what it does not know is visible rather than merely missing. """ import ast +import hashlib import os from dataclasses import dataclass +from pathlib import Path from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls, Unanalyzed from typing import Optional, Union + +def _version() -> str: + """What this analysis is, as a hash of the code that performs it. + + Results an older dashboard wrote are only worth keeping if the + analysis that wrote them was this one; hashing the source is what + makes that true without anyone having to remember to raise a + number when they change how the analysis works. + + Empty when the source cannot be read, which no result matches, so + everything is read again rather than trusted. + """ + try: + return hashlib.sha256(Path(__file__).read_bytes()).hexdigest() + except OSError: + return '' + + +VERSION = _version() + # The suffix a generated Python module carries. A name imported from # one is a state type, and the package it came from qualifies it. GENERATED_SUFFIX = '_rbt' @@ -125,6 +147,18 @@ class _Servicer: _Function = Union[ast.FunctionDef, ast.AsyncFunctionDef] +@dataclass(frozen=True) +class Analysis: + """What one method calls, and what that answer depended on. + + `hashes` holds the method's own hash and that of every function the + analysis followed into, so that a change to a helper in another + file invalidates this as surely as a change to the method itself. + """ + method_calls: MethodCalls + hashes: dict[str, str] + + def method_key(state_type: str, method: str) -> str: """Names one method the way `Analysis`es are keyed.""" return f'{state_type}.{method}' @@ -151,6 +185,25 @@ def _strip_docstrings(tree: ast.AST) -> None: node.body = _docstring_stripped(node.body) +def hash_function(function: _Function) -> str: + """What a function does, as a hash. + + `include_attributes=False` leaves out line and column numbers, so + reflowing an argument list or moving a method down a file gives + back the same hash; comments never reach the tree at all, and + docstrings are stripped when the file is parsed. + """ + parts = [ + function.name, + ast.dump(function.args, include_attributes=False), + ] + parts.extend( + ast.dump(statement, include_attributes=False) + for statement in function.body + ) + return hashlib.sha256('\n'.join(parts).encode()).hexdigest() + + @dataclass class Module: """One of the developer's source files, parsed.""" @@ -354,6 +407,7 @@ def __init__(self, modules: dict[str, Module]): self._modules = modules self._calls: list[Call] = [] self._unanalyzed: list[Unanalyzed] = [] + self._hashes: dict[str, str] = {} # Functions being followed right now, so that a helper that # calls itself, or two that call each other, stop rather than @@ -365,16 +419,22 @@ def analyze( module: Module, servicer: _Servicer, method: _Function, - ) -> MethodCalls: + ) -> Analysis: + self._hashes[_function_key(module, method, + servicer.name)] = hash_function(method) + environment = self._parameters(method, servicer) self._statements(method.body, module, environment) - return MethodCalls( - state_type=servicer.state_type, - method=method.name, - calls=_unique(self._calls), - unanalyzed=_unique(self._unanalyzed), + return Analysis( + method_calls=MethodCalls( + state_type=servicer.state_type, + method=method.name, + calls=_unique(self._calls), + unanalyzed=_unique(self._unanalyzed), + ), + hashes=dict(self._hashes), ) def _parameters( @@ -933,6 +993,8 @@ def _applications_index(self): if key in self._following: return None + self._hashes[key] = hash_function(function) + self._following.add(key) try: return self._expression( @@ -989,6 +1051,8 @@ def _follow( if key in self._following: return + self._hashes[key] = hash_function(function) + self._following.add(key) try: self._statements( @@ -1102,9 +1166,22 @@ def _unique(messages: list) -> list: return unique -def analyze(modules: dict[str, Module]) -> dict[str, MethodCalls]: - """Analyzes every servicer method in a tree of parsed files.""" - analyses: dict[str, MethodCalls] = {} +def analyze( + modules: dict[str, Module], + cache: Optional[dict[str, Analysis]] = None, +) -> dict[str, Analysis]: + """Analyzes every servicer method in a tree of parsed files. + + A method whose own hash and whose followed functions' hashes are + all as `cache` last saw them is taken from `cache` rather than + analyzed again, which is what keeps a keystroke from costing a + whole tree. + """ + cache = cache or {} + + hashes = _tree_hashes(modules) + + analyses: dict[str, Analysis] = {} for module in modules.values(): for class_definition in module.classes.values(): @@ -1136,6 +1213,11 @@ def analyze(modules: dict[str, Module]) -> dict[str, MethodCalls]: key = method_key(state_type, name) + cached = cache.get(key) + if cached is not None and _unchanged(cached, hashes): + analyses[key] = cached + continue + analyses[key] = _MethodAnalyzer(modules).analyze( module, servicer, statement ) @@ -1143,6 +1225,31 @@ def analyze(modules: dict[str, Module]) -> dict[str, MethodCalls]: return analyses +def _tree_hashes(modules: dict[str, Module]) -> dict[str, str]: + """Every function in the tree, hashed, so that what changed can be + told from what did not.""" + hashes: dict[str, str] = {} + + for module in modules.values(): + for function in module.functions.values(): + hashes[_function_key(module, function)] = hash_function(function) + for class_definition in module.classes.values(): + for statement in class_definition.body: + match statement: + case ast.FunctionDef() | ast.AsyncFunctionDef(): + hashes[_function_key( + module, statement, class_definition.name + )] = hash_function(statement) + + return hashes + + +def _unchanged(analysis: Analysis, hashes: dict[str, str]) -> bool: + return all( + hashes.get(key) == value for key, value in analysis.hashes.items() + ) + + def module_name(filename: str) -> str: """The dotted name a source file goes by, relative to the source directory it was found in.""" diff --git a/tests/reboot/dashboard/call_analysis_tests.py b/tests/reboot/dashboard/call_analysis_tests.py index dc7ecfe3..827f060e 100644 --- a/tests/reboot/dashboard/call_analysis_tests.py +++ b/tests/reboot/dashboard/call_analysis_tests.py @@ -4,8 +4,8 @@ to exist, and the file may be half written. Only the source. """ import unittest -from rbt.dashboard.v1.dashboard_pb2 import Call, MethodCalls, Unanalyzed -from reboot.dashboard.call_analysis import analyze, parse +from rbt.dashboard.v1.dashboard_pb2 import Call, Unanalyzed +from reboot.dashboard.call_analysis import Analysis, analyze, parse # Every servicer below is written against this, so that the tests read # as the developer's files do: a state class imported from a generated @@ -20,24 +20,24 @@ def _modules(**sources: str) -> dict: return {name: parse(name, source) for name, source in sources.items()} -def _analyze(**sources: str) -> dict[str, MethodCalls]: +def _analyze(**sources: str) -> dict[str, Analysis]: return analyze(_modules(**sources)) -def _calls(analyses: dict[str, MethodCalls], +def _calls(analyses: dict[str, Analysis], key: str) -> list[tuple[str, str, str]]: """What one method calls, as tuples that read like the source.""" return [ (call.state_type, call.method, Call.How.Name(call.how)) - for call in analyses[key].calls + for call in analyses[key].method_calls.calls ] -def _unanalyzed(analyses: dict[str, MethodCalls], +def _unanalyzed(analyses: dict[str, Analysis], key: str) -> list[tuple[str, str]]: return [ (Unanalyzed.Why.Name(entry.why), entry.expression) - for entry in analyses[key].unanalyzed + for entry in analyses[key].method_calls.unanalyzed ] @@ -509,6 +509,153 @@ async def ordinary(self, context, request): self.assertEqual(_calls(analyses, 'bank.v1.Account.ordinary'), []) self.assertEqual(_unanalyzed(analyses, 'bank.v1.Account.ordinary'), []) + ################################################################### + # Analyzing again only what changed. + + def _cached(self, first: dict, second: dict, key: str) -> bool: + return second[key] is first[key] + + def _servicer_calling(self, method: str) -> tuple[str, str]: + """A servicer whose method calls a helper in another file, + and that other file.""" + return _servicer( + ''' + async def move(self, context, request): + await transfer(context, request.amount) +''' + ).replace( + 'from bank.v1.bank_rbt import Bank', + 'from helpers import transfer', + ), IMPORTS + f''' + +async def transfer(context, amount): + await Account.ref('a').{method}(context, amount=amount) +''' + + def test_a_comment_or_a_reflow_changes_nothing(self) -> None: + servicer, helpers = self._servicer_calling('withdraw') + + first = analyze(_modules(servicer=servicer, helpers=helpers)) + + rewritten = servicer.replace( + 'await transfer(context, request.amount)', + '# Move the money.\n await transfer(\n' + ' context,\n request.amount,\n )', + ) + + second = analyze( + _modules(servicer=rewritten, helpers=helpers), cache=first + ) + + self.assertTrue(self._cached(first, second, 'bank.v1.Account.move')) + + def test_a_docstring_changes_nothing(self) -> None: + servicer, helpers = self._servicer_calling('withdraw') + + first = analyze(_modules(servicer=servicer, helpers=helpers)) + + documented = servicer.replace( + 'async def move(self, context, request):', + 'async def move(self, context, request):\n' + ' """Moves money between two accounts."""', + ) + + second = analyze( + _modules(servicer=documented, helpers=helpers), cache=first + ) + + self.assertTrue(self._cached(first, second, 'bank.v1.Account.move')) + + def test_a_helper_in_another_file_changing_is_noticed(self) -> None: + """The method itself is untouched, so its own hash is no help; + what invalidates it is the hash of what it followed into.""" + servicer, helpers = self._servicer_calling('withdraw') + + first = analyze(_modules(servicer=servicer, helpers=helpers)) + + _, changed = self._servicer_calling('deposit') + + second = analyze( + _modules(servicer=servicer, helpers=changed), cache=first + ) + + self.assertFalse(self._cached(first, second, 'bank.v1.Account.move')) + self.assertEqual( + _calls(second, 'bank.v1.Account.move'), + [('bank.v1.Account', 'deposit', 'CALL')], + ) + + def test_a_change_two_helpers_away_is_noticed(self) -> None: + """A method's answer depends on everything the analysis walked + through to reach it, however far that went, so what is recorded + is the hash of each of those in turn rather than only the ones + the method names itself.""" + servicer = _servicer( + ''' + async def move(self, context, request): + await f(context) +''' + ).replace('from bank.v1.bank_rbt import Bank', 'from middle import f') + + middle = 'from deep import g\n\n\nasync def f(context):\n' \ + ' await g(context)\n' + + def deep(method: str) -> str: + return IMPORTS + f''' + +async def g(context): + await Account.ref('a').{method}(context) +''' + + def modules(method: str) -> dict: + return _modules( + servicer=servicer, middle=middle, deep=deep(method) + ) + + first = analyze(modules('withdraw')) + + self.assertEqual( + _calls(first, 'bank.v1.Account.move'), + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + + # Everything walked through is recorded, not just what the + # method names. + self.assertEqual( + sorted(first['bank.v1.Account.move'].hashes), + ['deep:g', 'middle:f', 'servicer:AccountServicer.move'], + ) + + # `deep` is two hops from the method, and neither the method + # nor `middle` is touched. + second = analyze(modules('deposit'), cache=first) + + self.assertFalse(self._cached(first, second, 'bank.v1.Account.move')) + self.assertEqual( + _calls(second, 'bank.v1.Account.move'), + [('bank.v1.Account', 'deposit', 'CALL')], + ) + + def test_changing_what_a_method_calls_is_noticed(self) -> None: + servicer, helpers = self._servicer_calling('withdraw') + + first = analyze(_modules(servicer=servicer, helpers=helpers)) + + changed = servicer.replace( + 'await transfer(context, request.amount)', + 'await Account.ref(request.id).deposit(context)', + ) + + second = analyze( + _modules(servicer=changed, helpers=helpers), cache=first + ) + + self.assertFalse(self._cached(first, second, 'bank.v1.Account.move')) + self.assertEqual( + _calls(second, 'bank.v1.Account.move'), + [('bank.v1.Account', 'deposit', 'CALL')], + ) + if __name__ == '__main__': unittest.main() From a72dbcb651a298299941da11e9b44d9796295e02 Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:14:30 +0000 Subject: [PATCH 10/14] Read a directory of servicers off disk Turns the analysis of one parsed tree into the analysis of one directory: every file the developer wrote, generated code left out. The whole tree at once rather than a file at a time, because a call crosses files -- a method may reach a state through a helper written somewhere else entirely, and reading that helper's file alone would say nothing about the method that calls it. A file that will not parse is named rather than raised: a half-written file is the normal case while somebody is typing, so the files that did parse are still analyzed and the rest are reported. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- reboot/dashboard/BUILD.bazel | 11 ++ reboot/dashboard/servicer_reader.py | 87 ++++++++++++ tests/reboot/dashboard/BUILD.bazel | 9 ++ .../reboot/dashboard/servicer_reader_tests.py | 127 ++++++++++++++++++ 4 files changed, 234 insertions(+) create mode 100644 reboot/dashboard/servicer_reader.py create mode 100644 tests/reboot/dashboard/servicer_reader_tests.py diff --git a/reboot/dashboard/BUILD.bazel b/reboot/dashboard/BUILD.bazel index 64186eec..62a22c99 100644 --- a/reboot/dashboard/BUILD.bazel +++ b/reboot/dashboard/BUILD.bazel @@ -43,6 +43,17 @@ py_library( ], ) +py_library( + name = "servicer_reader_py", + srcs = ["servicer_reader.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":call_analysis_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + ], +) + py_library( name = "servicers_py", srcs = ["servicers.py"], diff --git a/reboot/dashboard/servicer_reader.py b/reboot/dashboard/servicer_reader.py new file mode 100644 index 00000000..61ad5837 --- /dev/null +++ b/reboot/dashboard/servicer_reader.py @@ -0,0 +1,87 @@ +"""Describes what the developer's methods call. + +Reads every source file in a directory and analyzes the whole tree at +once, because a call crosses files: a method may reach a state through +a helper written somewhere else entirely. What keeps that affordable is +that only methods whose code actually changed are analyzed again; see +`reboot.dashboard.call_analysis`. +""" +import os +from pathlib import Path +from rbt.dashboard.v1.dashboard_pb2 import MethodCalls +from reboot.dashboard.call_analysis import ( + Analysis, + Module, + analyze, + module_name, + parse, +) +from typing import Optional + +SOURCE_GLOB = '**/*.py' + +# What `rbt generate` writes, which the developer did not. The same +# list is in `reboot/cli/commands/generate.py`, in +# `reboot/cli/commands/dev.py` and in `reboot/dashboard/api_watcher.py`; +# keep them in step. +GENERATED_SUFFIXES = ('_rbt.py', '_pb2.py', '_pb2_grpc.py') + + +def _files(source_directory: Path) -> list[str]: + """Every file the developer wrote, relative to the directory.""" + return sorted( + str(path.relative_to(source_directory)) + for path in source_directory.glob(SOURCE_GLOB) + if path.is_file() and not str(path).endswith(GENERATED_SUFFIXES) + ) + + +def read( + source_directory: str, + cache: Optional[dict[str, Analysis]] = None, +) -> tuple[dict[str, Analysis], str]: + """Analyzes the methods in one directory of source files. + + Returns what each method calls, and a message naming the files that + could not be read. A half-written file is the normal case while + somebody is typing, so the files that did parse are still analyzed + and the rest are reported. + """ + directory = Path(source_directory).resolve() + + if not directory.is_dir(): + # Worth saying rather than showing an application that calls + # nothing: a directory that is not there is a typo, or one that + # has yet to be made. + return {}, f'{source_directory}: no such directory' + + modules: dict[str, Module] = {} + errors: list[str] = [] + + for filename in _files(directory): + name = module_name(filename) + + try: + modules[name] = parse(name, (directory / filename).read_text()) + except SyntaxError as e: + errors.append( + f'{os.path.join(source_directory, filename)}: ' + f'line {e.lineno}: {e.msg}' + ) + except OSError as e: + errors.append( + f'{os.path.join(source_directory, filename)}: {e.strerror}' + ) + + return analyze(modules, cache), '\n'.join(errors) + + +def method_calls(analyses: dict[str, Analysis]) -> list[MethodCalls]: + """What every method calls, in a settled order, so that reading + the same tree twice writes the same thing twice.""" + return [ + analyses[key].method_calls + for key in sorted(analyses) + if len(analyses[key].method_calls.calls) > 0 or + len(analyses[key].method_calls.unanalyzed) > 0 + ] diff --git a/tests/reboot/dashboard/BUILD.bazel b/tests/reboot/dashboard/BUILD.bazel index e36f059e..de199ad9 100644 --- a/tests/reboot/dashboard/BUILD.bazel +++ b/tests/reboot/dashboard/BUILD.bazel @@ -31,6 +31,15 @@ py_test( ], ) +py_test( + name = "servicer_reader_tests_py", + srcs = ["servicer_reader_tests.py"], + main = "servicer_reader_tests.py", + deps = [ + "//reboot/dashboard:servicer_reader_py", + ], +) + py_test( name = "application_tests_py", srcs = [":application_tests.py"], diff --git a/tests/reboot/dashboard/servicer_reader_tests.py b/tests/reboot/dashboard/servicer_reader_tests.py new file mode 100644 index 00000000..66535aa5 --- /dev/null +++ b/tests/reboot/dashboard/servicer_reader_tests.py @@ -0,0 +1,127 @@ +"""The dashboard reads a directory of servicers off disk. + +Nothing is imported and nothing is generated: `rbt generate` has not +run, there is no application, and the files are read as they are. +""" +import tempfile +import unittest +from pathlib import Path +from rbt.dashboard.v1.dashboard_pb2 import Call +from reboot.dashboard.servicer_reader import method_calls, read + +SERVICER = ''' +from bank.v1.account_rbt import Account +from helpers import transfer +from reboot.aio.contexts import WriterContext + + +class AccountServicer(Account.Servicer): + + def authorizer(self): + return allow() + + async def move(self, context: WriterContext, request): + await transfer(context, request.amount) + + async def quiet(self, context: WriterContext): + self.state.balance = 0 +''' + +HELPERS = ''' +from bank.v1.account_rbt import Account + + +async def transfer(context, amount): + await Account.ref('a').withdraw(context, amount=amount) +''' + + +class ServicerReaderTest(unittest.TestCase): + + def setUp(self) -> None: + self._directory = tempfile.TemporaryDirectory() + self.directory = Path(self._directory.name) + + def tearDown(self) -> None: + self._directory.cleanup() + + def _write(self, name: str, source: str) -> None: + (self.directory / name).write_text(source) + + def test_describes_what_a_method_calls(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + + analyses, error = read(str(self.directory)) + + self.assertEqual(error, '') + + described = method_calls(analyses) + + self.assertEqual( + [(one.state_type, one.method) for one in described], + [('bank.v1.Account', 'move')], + ) + self.assertEqual( + [ + (call.state_type, call.method, Call.How.Name(call.how)) + for call in described[0].calls + ], + [('bank.v1.Account', 'withdraw', 'CALL')], + ) + + def test_a_method_that_calls_nothing_is_left_out(self) -> None: + """A row saying a method calls nothing is a row saying + nothing.""" + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + + analyses, _ = read(str(self.directory)) + + self.assertNotIn( + 'quiet', + [one.method for one in method_calls(analyses)], + ) + + def test_generated_files_are_not_read(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + self._write('account_rbt.py', 'this is not Python at all') + + _, error = read(str(self.directory)) + + self.assertEqual(error, '') + + def test_a_half_written_file_is_reported_and_the_rest_still_read( + self + ) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + self._write('halfway.py', 'async def transfer(context,') + + analyses, error = read(str(self.directory)) + + self.assertIn('halfway.py', error) + self.assertEqual( + [one.method for one in method_calls(analyses)], + ['move'], + ) + + def test_a_directory_that_is_not_there_is_said_so(self) -> None: + analyses, error = read(str(self.directory / 'nowhere')) + + self.assertEqual(analyses, {}) + self.assertIn('no such directory', error) + + def test_reading_twice_says_the_same_thing(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + + first, _ = read(str(self.directory)) + second, _ = read(str(self.directory), first) + + self.assertEqual(method_calls(first), method_calls(second)) + + +if __name__ == '__main__': + unittest.main() From 9964b8bb1c91c0c8392612a2e613052b21cf71a4 Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:15:07 +0000 Subject: [PATCH 11/14] Teach `rbt dashboard` where the developer's servicers are The dashboard knows where the API files are, and nothing else about the developer's tree. But what a method calls is written where the method is implemented, not where its API is declared, and those are different directories: `api/` against `backend/src/`. So `--source-directory`, spelled and passed the way `--api-directory` already is, so that a file can be shown as `backend/src/bank_servicer.py`. Optional, unlike `--api-directory`: an existing invocation keeps working, and a Node.js application, whose servicers this cannot read, is not made to name a directory that buys it nothing. Nothing reads the variable yet; the next commit does. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- reboot/cli/commands/dashboard.py | 19 ++++++++++ reboot/dashboard/constants.py | 7 ++++ tests/reboot/cli/dashboard_tests.py | 59 ++++++++++++++++++++++++----- 3 files changed, 76 insertions(+), 9 deletions(-) diff --git a/reboot/cli/commands/dashboard.py b/reboot/cli/commands/dashboard.py index 35f51ea0..6850559e 100644 --- a/reboot/cli/commands/dashboard.py +++ b/reboot/cli/commands/dashboard.py @@ -23,6 +23,7 @@ DASHBOARD_PATH, DEFAULT_DASHBOARD_PORT, ENVVAR_RBT_API_DIRECTORY, + ENVVAR_RBT_SOURCE_DIRECTORY, ) from reboot.settings import ( ENVVAR_RBT_DEV, @@ -63,6 +64,14 @@ def register_dashboard(parser: ArgumentParser): help='directory containing the API files the dashboard watches', ) + parser.subcommand('dashboard').add_argument( + '--source-directory', + type=str, + required=False, + help='directory containing the servicers the dashboard analyzes, ' + 'to show what each method calls; without it, no calls are shown', + ) + parser.subcommand('dashboard').add_argument( '--port', type=int, @@ -77,6 +86,7 @@ def _dashboard_env( *, port: int, api_directory: str, + source_directory: Optional[str], ) -> dict[str, str]: """The environment for the dashboard application. @@ -118,6 +128,14 @@ def _dashboard_env( # directory, where that spelling resolves. composed[ENVVAR_RBT_API_DIRECTORY] = api_directory + # Where the developer's servicers are, spelled the same way and + # for the same reason. Left out of the environment entirely when + # the developer named no directory, which is what tells the + # dashboard there is nothing to analyze. + composed.pop(ENVVAR_RBT_SOURCE_DIRECTORY, None) + if source_directory is not None: + composed[ENVVAR_RBT_SOURCE_DIRECTORY] = source_directory + composed[ENVVAR_RBT_NAME] = DASHBOARD_STATE_DIRECTORY_NAME state_directory = ( @@ -219,6 +237,7 @@ async def dashboard( parser, port=port, api_directory=args.api_directory, + source_directory=args.source_directory, ) terminal.info( diff --git a/reboot/dashboard/constants.py b/reboot/dashboard/constants.py index 5a16d3b9..233aeebe 100644 --- a/reboot/dashboard/constants.py +++ b/reboot/dashboard/constants.py @@ -36,6 +36,13 @@ # is serving, and the dashboard is meant to be startable that early. ENVVAR_RBT_API_DIRECTORY = 'RBT_API_DIRECTORY' +# The directory the developer's servicers are in, which `rbt dashboard` +# takes as `--source-directory`. Separate from the API directory +# because what a method calls is written where the method is +# implemented, not where its API is declared. Unset when the developer +# named no such directory, in which case nothing is analyzed. +ENVVAR_RBT_SOURCE_DIRECTORY = 'RBT_SOURCE_DIRECTORY' + # The `Presence` state the dashboard page subscribes to, recording who # is looking at a dashboard right now. `rbt dev run` reads it to decide # whether to open one. diff --git a/tests/reboot/cli/dashboard_tests.py b/tests/reboot/cli/dashboard_tests.py index f5bea4ff..b3910657 100644 --- a/tests/reboot/cli/dashboard_tests.py +++ b/tests/reboot/cli/dashboard_tests.py @@ -10,21 +10,27 @@ MockExitException, mock_raise_instead_of_exit, ) +from typing import Optional from unittest.mock import patch @patch('argparse.ArgumentParser.exit', mock_raise_instead_of_exit) class RbtDashboardTestCase(unittest.IsolatedAsyncioTestCase): - def _parse(self, state_directory: str): - parser: ArgumentParser = cli.create_parser( - argv=[ - 'rbt', - f'--state-directory={state_directory}', - 'dashboard', - '--api-directory=api', - ] - ) + def _parse( + self, + state_directory: str, + source_directory: Optional[str] = None, + ): + argv = [ + 'rbt', + f'--state-directory={state_directory}', + 'dashboard', + '--api-directory=api', + ] + if source_directory is not None: + argv.append(f'--source-directory={source_directory}') + parser: ArgumentParser = cli.create_parser(argv=argv) args, _ = parser.parse_args() return args, parser @@ -55,6 +61,7 @@ async def test_env_is_isolated_from_any_application(self) -> None: parser, port=DEFAULT_DASHBOARD_PORT, api_directory=args.api_directory, + source_directory=args.source_directory, ) self.assertEqual(env['RBT_NAME'], 'dashboard') @@ -89,6 +96,7 @@ async def test_keys_differ_from_any_application(self) -> None: parser, port=DEFAULT_DASHBOARD_PORT, api_directory=args.api_directory, + source_directory=args.source_directory, ) self.assertNotEqual(env['REBOOT_CRYPTO_ROOT_KEYS'], 'v1:theirs') @@ -100,6 +108,7 @@ async def test_keys_differ_from_any_application(self) -> None: parser, port=DEFAULT_DASHBOARD_PORT, api_directory=args.api_directory, + source_directory=args.source_directory, ) self.assertEqual( env['REBOOT_CRYPTO_ROOT_KEYS'], @@ -115,6 +124,7 @@ async def test_is_told_where_the_api_files_are(self) -> None: parser, port=DEFAULT_DASHBOARD_PORT, api_directory=args.api_directory, + source_directory=args.source_directory, ) # As the developer spelled it, so files can be shown as @@ -122,6 +132,37 @@ async def test_is_told_where_the_api_files_are(self) -> None: # working directory where that spelling resolves. self.assertEqual(env['RBT_API_DIRECTORY'], 'api') + async def test_is_told_where_the_servicers_are(self) -> None: + with tempfile.TemporaryDirectory() as state_directory: + args, parser = self._parse(state_directory, 'backend/src') + + env = dashboard._dashboard_env( + args, + parser, + port=DEFAULT_DASHBOARD_PORT, + source_directory=args.source_directory, + api_directory=args.api_directory, + ) + + self.assertEqual(env['RBT_SOURCE_DIRECTORY'], 'backend/src') + + async def test_a_source_directory_is_not_required(self) -> None: + """Somebody who names none gets a dashboard without calls, + rather than an error; a Node.js application has no directory + this could read.""" + with tempfile.TemporaryDirectory() as state_directory: + args, parser = self._parse(state_directory) + + env = dashboard._dashboard_env( + args, + parser, + port=DEFAULT_DASHBOARD_PORT, + source_directory=args.source_directory, + api_directory=args.api_directory, + ) + + self.assertNotIn('RBT_SOURCE_DIRECTORY', env) + if __name__ == '__main__': unittest.main() From 300793d54ac2effa4b6167a2410cf9f6bf333307 Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:16:31 +0000 Subject: [PATCH 12/14] Publish what each method calls, as the source changes Watches the source directory the way `Watch` already watches the API directory, and writes what it finds into the dashboard's own state, so that a browser can read it without reaching the application being developed -- which need not be running, or even built. Its own workflow rather than more of `Watch`, for two reasons: a workflow may have only one loop, and a developer who named no source directory leaves this one with nothing to do while `Watch` still has its files. Its own writer and its own pair of fields on `API`, rather than a field inside `StateTypeInfo`, because a different watcher on a different directory produces it. One writer replacing the other's field would lose whichever wrote first -- the shape `Preferences` already uses for the same reason. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- rbt/dashboard/v1/dashboard.proto | 40 ++++++ reboot/dashboard/BUILD.bazel | 16 +++ reboot/dashboard/main.py | 3 + reboot/dashboard/servicer_watcher.py | 53 +++++++ reboot/dashboard/servicers.py | 39 ++++- tests/reboot/dashboard/BUILD.bazel | 11 ++ .../dashboard/servicer_watcher_tests.py | 136 ++++++++++++++++++ 7 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 reboot/dashboard/servicer_watcher.py create mode 100644 tests/reboot/dashboard/servicer_watcher_tests.py diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto index 564d20b5..003b874c 100644 --- a/rbt/dashboard/v1/dashboard.proto +++ b/rbt/dashboard/v1/dashboard.proto @@ -133,6 +133,18 @@ message API { // half-written file is the normal case while someone is typing, and // saying so beats showing nothing. string error = 2; + + // What each method's implementation calls, read from the + // developer's source files rather than from their API files. + // + // Held beside `state_types` rather than inside them, with its own + // writer, because a different watcher on a different directory + // produces it; one writer replacing the other's field would lose + // whichever wrote first. + repeated MethodCalls method_calls = 3; + + // Why the source files could not be read, if they could not be. + string calls_error = 4; } message APIGetRequest {} @@ -140,6 +152,8 @@ message APIGetRequest {} message APIGetResponse { repeated StateTypeInfo state_types = 1; string error = 2; + repeated MethodCalls method_calls = 3; + string calls_error = 4; } message APIUpdateRequest { @@ -149,10 +163,21 @@ message APIUpdateRequest { message APIUpdateResponse {} +message APIUpdateCallsRequest { + repeated MethodCalls method_calls = 1; + string error = 2; +} + +message APIUpdateCallsResponse {} + message APIWatchRequest {} message APIWatchResponse {} +message APIWatchCallsRequest {} + +message APIWatchCallsResponse {} + //////////////////////////////////////////////////////////////////////// // What the developer has told the dashboard about opening dashboards. @@ -221,12 +246,27 @@ service APIMethods { }; } + rpc UpdateCalls(APIUpdateCallsRequest) returns (APIUpdateCallsResponse) { + option (rbt.v1alpha1.method).writer = { + }; + } + // Watches the developer's API files for as long as the dashboard // application runs, reading each one that changes. rpc Watch(APIWatchRequest) returns (APIWatchResponse) { option (rbt.v1alpha1.method).workflow = { }; } + + // Watches the developer's source files the same way, analyzing what + // their methods call. Its own workflow rather than more of `Watch` + // because a workflow may only have one loop, and because a source + // directory the developer never named leaves this one with nothing + // to do while `Watch` still has its files. + rpc WatchCalls(APIWatchCallsRequest) returns (APIWatchCallsResponse) { + option (rbt.v1alpha1.method).workflow = { + }; + } } //////////////////////////////////////////////////////////////////////// diff --git a/reboot/dashboard/BUILD.bazel b/reboot/dashboard/BUILD.bazel index 62a22c99..a7d6acbd 100644 --- a/reboot/dashboard/BUILD.bazel +++ b/reboot/dashboard/BUILD.bazel @@ -54,6 +54,21 @@ py_library( ], ) +py_library( + name = "servicer_watcher_py", + srcs = ["servicer_watcher.py"], + srcs_version = "PY3", + visibility = ["//visibility:public"], + deps = [ + ":call_analysis_py", + ":constants_py", + ":servicer_reader_py", + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot/aio:external_py", + "//reboot/cli/common:watch_py", + ], +) + py_library( name = "servicers_py", srcs = ["servicers.py"], @@ -62,6 +77,7 @@ py_library( deps = [ ":api_watcher_py", ":constants_py", + ":servicer_watcher_py", "//rbt/dashboard/v1:dashboard_py_reboot", "//reboot/aio:servicers_py", "//reboot/std/presence/v1:presence_py", diff --git a/reboot/dashboard/main.py b/reboot/dashboard/main.py index fdfc9e1b..7f76c3f9 100644 --- a/reboot/dashboard/main.py +++ b/reboot/dashboard/main.py @@ -75,6 +75,9 @@ async def initialize(context: InitializeContext) -> None: # watcher. _ = await API.ref(API_ID).idempotently('watch').spawn().Watch(context) + _ = await API.ref(API_ID).idempotently('watch calls' + ).spawn().WatchCalls(context) + async def main(): await application().run() diff --git a/reboot/dashboard/servicer_watcher.py b/reboot/dashboard/servicer_watcher.py new file mode 100644 index 00000000..f092ed03 --- /dev/null +++ b/reboot/dashboard/servicer_watcher.py @@ -0,0 +1,53 @@ +"""Keeps what the developer's methods call up to date. + +The same shape as `reboot.dashboard.api_watcher`, over the source +directory rather than the API directory. It differs in reading the +whole tree on every change rather than only the files that changed, +because a call crosses files: editing a helper changes what its +callers call. `reboot.dashboard.call_analysis` is what keeps that +cheap, by analyzing again only the methods whose code actually +changed. +""" +from pathlib import Path +from rbt.dashboard.v1.dashboard_rbt import API +from reboot.aio.contexts import WorkflowContext +from reboot.cli.common.watch import file_watcher +from reboot.dashboard.call_analysis import Analysis +from reboot.dashboard.constants import API_ID +from reboot.dashboard.servicer_reader import SOURCE_GLOB, method_calls, read +from typing import Optional + + +async def watch(context: WorkflowContext, *, source_directory: str) -> None: + """Updates what the methods call, for as long as this runs.""" + directory = Path(source_directory).resolve() + + analyses: dict[str, Analysis] = {} + updated: Optional[tuple] = None + + with file_watcher() as watcher: + async for iteration in context.loop('analyze what changed'): + # The watch is armed before anything is read, so a save + # made during an analysis is not missed: it resolves + # `event` rather than arriving while nothing is listening. + # A watch is consumed by one event, so it is re-entered for + # each. + async with watcher.watch( + [SOURCE_GLOB], + root_dir=str(directory), + ) as event: + analyses, error = read(source_directory, analyses) + + current = (method_calls(analyses), error) + + if current != updated: + updated = current + # Every write from a workflow needs an identity, + # and this one writes at most once an iteration. + await API.ref(API_ID).per_iteration('analyze').UpdateCalls( + context, + method_calls=current[0], + error=current[1], + ) + + await event diff --git a/reboot/dashboard/servicers.py b/reboot/dashboard/servicers.py index dec9cd41..0092e764 100644 --- a/reboot/dashboard/servicers.py +++ b/reboot/dashboard/servicers.py @@ -4,6 +4,8 @@ from rbt.dashboard.v1.dashboard_pb2 import ( APIGetRequest, APIGetResponse, + APIUpdateCallsRequest, + APIUpdateCallsResponse, APIUpdateRequest, APIUpdateResponse, PreferencesGetRequest, @@ -18,7 +20,11 @@ from reboot.aio.contexts import ReaderContext, WorkflowContext, WriterContext from reboot.aio.servicers import Servicer from reboot.dashboard.api_watcher import watch -from reboot.dashboard.constants import ENVVAR_RBT_API_DIRECTORY +from reboot.dashboard.constants import ( + ENVVAR_RBT_API_DIRECTORY, + ENVVAR_RBT_SOURCE_DIRECTORY, +) +from reboot.dashboard.servicer_watcher import watch as watch_calls class APIServicer(API.Servicer): @@ -35,6 +41,8 @@ async def Get( return APIGetResponse( state_types=self.state.state_types, error=self.state.error, + method_calls=self.state.method_calls, + calls_error=self.state.calls_error, ) @classmethod @@ -56,6 +64,25 @@ async def Watch( return API.WatchResponse() + @classmethod + async def WatchCalls( + cls, + context: WorkflowContext, + request: API.WatchCallsRequest, + ) -> API.WatchCallsResponse: + """Analyzes what the developer's methods call when they change. + + A developer who named no source directory gets no analysis; + there is nowhere to read the implementations from, which is the + normal case for a Node.js application. + """ + source_directory = os.environ.get(ENVVAR_RBT_SOURCE_DIRECTORY) + + if source_directory is not None: + await watch_calls(context, source_directory=source_directory) + + return API.WatchCallsResponse() + async def Update( self, context: WriterContext, @@ -66,6 +93,16 @@ async def Update( self.state.error = request.error return APIUpdateResponse() + async def UpdateCalls( + self, + context: WriterContext, + request: APIUpdateCallsRequest, + ) -> APIUpdateCallsResponse: + del self.state.method_calls[:] + self.state.method_calls.extend(request.method_calls) + self.state.calls_error = request.error + return APIUpdateCallsResponse() + class PreferencesServicer(Preferences.Servicer): """Holds what the developer has said about their dashboard. diff --git a/tests/reboot/dashboard/BUILD.bazel b/tests/reboot/dashboard/BUILD.bazel index de199ad9..f9455818 100644 --- a/tests/reboot/dashboard/BUILD.bazel +++ b/tests/reboot/dashboard/BUILD.bazel @@ -40,6 +40,17 @@ py_test( ], ) +py_test( + name = "servicer_watcher_tests_py", + srcs = ["servicer_watcher_tests.py"], + main = "servicer_watcher_tests.py", + deps = [ + "//reboot/aio:tests_py", + "//reboot/dashboard:main_py", + "//reboot/dashboard:servicer_watcher_py", + ], +) + py_test( name = "application_tests_py", srcs = [":application_tests.py"], diff --git a/tests/reboot/dashboard/servicer_watcher_tests.py b/tests/reboot/dashboard/servicer_watcher_tests.py new file mode 100644 index 00000000..3cc5e4a2 --- /dev/null +++ b/tests/reboot/dashboard/servicer_watcher_tests.py @@ -0,0 +1,136 @@ +"""What the methods call follows the developer's source files. + +The API watcher shows what has been declared; this shows what has been +implemented, and keeps showing it as the implementation changes. +""" +import asyncio +import os +import tempfile +import unittest +from pathlib import Path +from rbt.dashboard.v1.dashboard_rbt import API +from reboot.aio.tests import Reboot +from reboot.dashboard.constants import ( + API_ID, + ENVVAR_RBT_API_DIRECTORY, + ENVVAR_RBT_SOURCE_DIRECTORY, +) +from reboot.dashboard.main import application +from typing import Optional +from unittest.mock import patch + +SERVICER = ''' +from bank.v1.account_rbt import Account +from helpers import transfer + + +class AccountServicer(Account.Servicer): + + def authorizer(self): + return allow() + + async def move(self, context, request): + await transfer(context, request.amount) +''' + +HELPERS = ''' +from bank.v1.account_rbt import Account + + +async def transfer(context, amount): + await Account.ref('a').{method}(context, amount=amount) +''' + + +class ServicerWatcherTest(unittest.IsolatedAsyncioTestCase): + + watcher: Optional[asyncio.Task] = None + + async def asyncSetUp(self) -> None: + # Both directories are read when the application comes up, so + # they have to exist and be named first. + self._api = tempfile.TemporaryDirectory() + self._source = tempfile.TemporaryDirectory() + self.source = Path(self._source.name) + + self._environment = patch.dict( + os.environ, + { + ENVVAR_RBT_API_DIRECTORY: self._api.name, + ENVVAR_RBT_SOURCE_DIRECTORY: str(self.source), + }, + ) + self._environment.start() + + self.rbt = Reboot() + await self.rbt.start() + await self.rbt.up(application(), local_envoy=True) + + async def asyncTearDown(self) -> None: + await self.rbt.stop() + self._environment.stop() + self._source.cleanup() + self._api.cleanup() + + def _write(self, name: str, source: str) -> None: + (self.source / name).write_text(source) + + async def _wait_for(self, satisfied): + while True: + context = self.rbt.create_external_context(name=self.id()) + try: + response = await API.ref(API_ID).Get(context) + if satisfied(response): + return response + except Exception: + pass + await asyncio.sleep(0.1) + + async def test_calls_follow_the_source(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS.format(method='withdraw')) + + response = await self._wait_for(lambda api: len(api.method_calls) == 1) + + self.assertEqual(response.calls_error, '') + self.assertEqual( + [ + (one.state_type, one.method, one.calls[0].method) + for one in response.method_calls + ], + [('bank.v1.Account', 'move', 'withdraw')], + ) + + # A change to the helper, in a file that is not the method's, + # changes what the method is said to call. + self._write('helpers.py', HELPERS.format(method='deposit')) + + response = await self._wait_for( + lambda api: len(api.method_calls) == 1 and api.method_calls[0]. + calls[0].method == 'deposit' + ) + + self.assertEqual( + [one.calls[0].method for one in response.method_calls], + ['deposit'], + ) + + async def test_a_half_written_file_is_reported(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS.format(method='withdraw')) + + await self._wait_for(lambda api: len(api.method_calls) == 1) + + self._write('halfway.py', 'async def transfer(context,') + + response = await self._wait_for(lambda api: api.calls_error != '') + + self.assertIn('halfway.py', response.calls_error) + + # What did parse is still shown; a file being typed into does + # not empty the page. + self.assertEqual(len(response.method_calls), 1) + + +if __name__ == '__main__': + unittest.main() From a6d56ca65dc6fb4cb9545be1139e94f7287d2038 Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:17:45 +0000 Subject: [PATCH 13/14] Show what each method calls on the dashboard Under each method's signature, which is what comes in and out, goes what the method sends on to the rest of the application: the state type and method it calls, and the verb that says how -- "calls", "schedules", "spawns", "watches", "waits on". Each call links to the state type it names, which the page already renders in a section carrying that name as its id, so the page reads as the graph it describes. What the analysis could not follow is shown after the calls, phrased as what is not known, so the list above never reads as everything there is. The block sits inside the detail that a state type's "Expand details" already opens, so it needs no state of its own. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- reboot/dashboard/frontend/dashboard.css | 44 ++++++++++ reboot/dashboard/frontend/src/main.tsx | 99 ++++++++++++++++++++++- tests/reboot/dashboard/dashboard_tests.py | 76 ++++++++++++++++- 3 files changed, 214 insertions(+), 5 deletions(-) diff --git a/reboot/dashboard/frontend/dashboard.css b/reboot/dashboard/frontend/dashboard.css index 99ce79fa..262f1718 100644 --- a/reboot/dashboard/frontend/dashboard.css +++ b/reboot/dashboard/frontend/dashboard.css @@ -705,3 +705,47 @@ header h1 { .errors { color: hsl(var(--errors)); } + +/* Sits flush under the signature and reads as its continuation: the + signature is what comes in and out, this is what goes on to the + rest of the application. */ +.method-calls { + display: flex; + flex-direction: column; + gap: 4px; + padding: 10px 18px; + background: hsl(var(--surface-sunken)); + border-top: 1px solid hsl(var(--border-soft)); + font-family: ui-monospace, Menlo, monospace; + font-size: 11.5px; + color: hsl(var(--muted-foreground)); +} + +.call-how { + color: hsl(240 3.8% 65%); +} + +.call-target { + color: hsl(var(--returns)); + text-decoration: none; +} + +.call-target:hover { + text-decoration: underline; +} + +.call-method { + color: hsl(var(--returns)); +} + +/* What could not be followed, in the same hue as an error without + being one: the analysis is saying it read less than everything, + not that anything is wrong. */ +.call-unanalyzed { + color: hsl(var(--errors)); + opacity: 0.85; +} + +.call-expression { + font-family: inherit; +} diff --git a/reboot/dashboard/frontend/src/main.tsx b/reboot/dashboard/frontend/src/main.tsx index 5c8a883f..1a39c5d8 100644 --- a/reboot/dashboard/frontend/src/main.tsx +++ b/reboot/dashboard/frontend/src/main.tsx @@ -1,4 +1,9 @@ -import type { MethodInfo, StateTypeInfo } from "@dashboard/dashboard_pb"; +import type { + MethodCalls, + MethodInfo, + StateTypeInfo, +} from "@dashboard/dashboard_pb"; +import { Call_How, Unanalyzed_Why } from "@dashboard/dashboard_pb"; import { useAPI, usePreferences } from "@dashboard/dashboard_rbt_react"; import { RebootClientProvider } from "@reboot-dev/reboot-react"; import { Presence } from "@reboot-dev/reboot-std-react/presence"; @@ -146,7 +151,64 @@ const Namespace: FC<{ namespace: string; types: StateTypeInfo[] }> = ({ ); }; -const Method: FC<{ method: MethodInfo }> = ({ method }) => { +// How a call is written, as the verb to put in front of it. What the +// developer wrote is a chain; what they meant is one of these. +const HOW: Record = { + [Call_How.UNKNOWN]: "calls", + [Call_How.CALL]: "calls", + [Call_How.CONSTRUCT]: "constructs", + [Call_How.SCHEDULE]: "schedules", + [Call_How.SPAWN]: "spawns", + [Call_How.REACTIVELY]: "watches", + [Call_How.UNTIL]: "waits on", + [Call_How.READ]: "reads", + [Call_How.WRITE]: "writes", +}; + +// What could not be followed, said as what it means for the list it +// sits under: not that nothing is called, but that this is not all of +// it. +const WHY: Record = { + [Unanalyzed_Why.UNKNOWN]: "could not be read", + [Unanalyzed_Why.CONTEXT_PASSED_TO_UNKNOWN_FUNCTION]: + "calls something this could not read", + [Unanalyzed_Why.REFERENCE_ESCAPED]: "keeps a reference this cannot follow", + [Unanalyzed_Why.UNKNOWN_METHOD]: "names its method only when it runs", +}; + +// What a method's implementation calls, read from the developer's +// source rather than declared in their API. +const Calls: FC<{ calls: MethodCalls }> = ({ calls }) => ( +
+ {calls.calls.map((call) => ( +
+ {HOW[call.how]}{" "} + {/* The section a state type is rendered in carries its full + name as an id, so a call is a link to what it calls. */} + + {typeNameOf(call.stateType)} + {call.method !== "" && ( + .{call.method} + )} + +
+ ))} + {calls.unanalyzed.map((entry) => ( +
+ {entry.expression}{" "} + {WHY[entry.why]} +
+ ))} +
+); + +const Method: FC<{ method: MethodInfo; calls?: MethodCalls }> = ({ + method, + calls, +}) => { const args = method.arguments .map((argument) => `${argument.name}: ${argument.type}`) .join(", "); @@ -211,6 +273,10 @@ const Method: FC<{ method: MethodInfo }> = ({ method }) => { raises {method.errors.join(", ")} )}
+ {/* After the signature, which is what comes in and out; + this is what goes back out to the rest of the + application. */} + {calls !== undefined && }
@@ -283,7 +349,8 @@ const StateType: FC<{ stateType: StateTypeInfo; expanded: boolean; onToggle: () => void; -}> = ({ stateType, expanded, onToggle }) => { + callsOf: (method: string) => MethodCalls | undefined; +}> = ({ stateType, expanded, onToggle, callsOf }) => { const section = useSlidingPills(expanded); return ( @@ -348,7 +415,11 @@ const StateType: FC<{
methods
{stateType.methods.map((method) => ( - + ))}
@@ -401,6 +472,25 @@ const Overview: FC<{ // typing, and saying so beats showing nothing. const error = response?.error ?? ""; + // What the developer's servicers call, read from their source files. + // Kept across a reconnect the same way the shape is, so a restart + // does not empty the calls out from under a page that still shows + // the methods making them. + const analyzed = response?.methodCalls; + const seenCalls = useRef([]); + + if (analyzed !== undefined && analyzed.length > 0) { + seenCalls.current = analyzed; + } + + const calls = useMemo(() => { + const byMethod = new Map(); + for (const one of analyzed?.length ? analyzed : seenCalls.current) { + byMethod.set(`${one.stateType}.${one.method}`, one); + } + return byMethod; + }, [analyzed]); + const namespaces = useMemo(() => { const byNamespace = new Map(); for (const stateType of stateTypes) { @@ -472,6 +562,7 @@ const Overview: FC<{ stateType={stateType} expanded={isExpanded(stateType.name)} onToggle={() => onToggle(stateType.name)} + callsOf={(method) => calls.get(`${stateType.name}.${method}`)} key={stateType.name} /> ))} diff --git a/tests/reboot/dashboard/dashboard_tests.py b/tests/reboot/dashboard/dashboard_tests.py index 67b1f6c7..a3bcce0f 100644 --- a/tests/reboot/dashboard/dashboard_tests.py +++ b/tests/reboot/dashboard/dashboard_tests.py @@ -8,7 +8,14 @@ import asyncio import socket import unittest -from rbt.dashboard.v1.dashboard_pb2 import FieldInfo, MethodInfo, StateTypeInfo +from rbt.dashboard.v1.dashboard_pb2 import ( + Call, + FieldInfo, + MethodCalls, + MethodInfo, + StateTypeInfo, + Unanalyzed, +) from rbt.dashboard.v1.dashboard_rbt import API, Preferences from reboot.aio.tests import Reboot from reboot.dashboard.constants import ( @@ -140,6 +147,39 @@ async def _record_state_types(self) -> None: error='', ) + async def _record_method_calls(self) -> None: + """Puts what analyzing a servicer would yield into the + application, for the same reason `_record_state_types` does.""" + context = self.rbt.create_external_context(name=self.id()) + await API.ref(API_ID).UpdateCalls( + context, + method_calls=[ + MethodCalls( + state_type='shop.v1.Shop', + method='look', + calls=[ + Call( + state_type='shop.v1.Depot', + method='stock', + how=Call.CALL, + ), + Call( + state_type='shop.v1.Depot', + method='restock', + how=Call.SCHEDULE, + ), + ], + unanalyzed=[ + Unanalyzed( + why=Unanalyzed.CONTEXT_PASSED_TO_UNKNOWN_FUNCTION, + expression='elsewhere.audit(context)', + ), + ], + ), + ], + error='', + ) + def _run(self, body): driver = _driver() try: @@ -184,6 +224,40 @@ def body(driver): self.assertIn('1 state type', page) self.assertIn('1 method', page) + async def test_shows_what_a_method_calls(self) -> None: + # What a method calls is read from the developer's source + # files, so it arrives separately from what their API files + # declare and is joined back onto the method here. + def body(driver): + driver.get(f'{self.url}{DASHBOARD_PATH}/') + WebDriverWait(driver, 60).until( + expected_conditions.presence_of_element_located( + (By.CLASS_NAME, 'method-calls') + ) + ) + return driver.page_source + + await self._record_state_types() + await self._record_method_calls() + + page = await asyncio.to_thread(self._run, body) + + # Each call says how it is reached, which is what tells a + # scheduled call from an awaited one. + self.assertIn('calls', page) + self.assertIn('schedules', page) + self.assertIn('stock', page) + self.assertIn('restock', page) + + # A call links to the state type it calls, which the page + # renders in a section carrying that name as its id. + self.assertIn('#shop.v1.Depot', page) + + # What could not be followed is shown too, so that the list + # above does not read as everything there is. + self.assertIn('elsewhere.audit(context)', page) + self.assertIn('calls something this could not read', page) + async def test_says_why_a_file_could_not_be_read(self) -> None: # A half-written file is the normal case while someone is # typing, so the page says what went wrong while keeping the From 02c27d4705421ddce8a7f898234c1759bb687f3e Mon Sep 17 00:00:00 2001 From: Benjamin Hindman Date: Sat, 15 Aug 2026 03:19:39 +0000 Subject: [PATCH 14/14] Keep the analysis across a dashboard restart A dashboard that starts again re-reads and re-analyzes a tree nobody has touched, which on the largest application here costs 84ms of parsing to arrive at exactly what the last one already knew. So a pass writes down what it saw -- each file's modified time and size, and per method the hashes it depended on -- beside what it concluded. A pass whose files are all as the last one left them says so before opening any of them, and costs 0.7ms. The timestamp is compared for being *different* rather than newer, because checking out an older branch moves a file's time backwards and that is as much an edit as any other; size comes along because two edits a moment apart can land on one timestamp. The two levels compose in one direction only, which is the safe one: a timestamp that changes when the code did not costs a parse and nothing more, because the method hashes then find the methods identical. That is why a timestamp is enough here and a content hash is not needed. None of it reaches `APIGetResponse`; a browser has no use for any of it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QPEVMhyDRxZEuH8eiykv99 --- rbt/dashboard/v1/dashboard.proto | 60 ++++++++ reboot/dashboard/servicer_reader.py | 119 ++++++++++++++-- reboot/dashboard/servicer_watcher.py | 103 ++++++++++++-- reboot/dashboard/servicers.py | 24 ++++ .../reboot/dashboard/servicer_reader_tests.py | 132 ++++++++++++++++-- .../dashboard/servicer_watcher_tests.py | 61 ++++++++ 6 files changed, 464 insertions(+), 35 deletions(-) diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto index 003b874c..6f004893 100644 --- a/rbt/dashboard/v1/dashboard.proto +++ b/rbt/dashboard/v1/dashboard.proto @@ -105,6 +105,33 @@ message Unanalyzed { string expression = 2; } +// One source file as the analysis last saw it, so that a dashboard +// starting again can tell which files it need not read. +message FileState { + // Relative to the source directory, e.g. `bank_servicer.py`. + string filename = 1; + + // Compared for being different rather than for being newer: + // checking out an older branch moves a file's time backwards, and + // that is as much an edit as any other. + uint64 modified_ns = 2; + + uint64 size = 3; +} + +// What one method's analysis depended on, so that it can be kept when +// none of those things has changed. +message MethodHashes { + string state_type = 1; + string method = 2; + + // The method's own hash and that of every function the analysis + // followed into, by `:`. A change anywhere in + // that set, however many calls away it is, is what makes the method + // worth analyzing again. + map hashes = 3; +} + // The calls one method's implementation makes. message MethodCalls { // The state type and method whose implementation this describes, @@ -145,6 +172,17 @@ message API { // Why the source files could not be read, if they could not be. string calls_error = 4; + + // What the analysis last read, so that a dashboard starting again + // reads only what has changed since. Deliberately not in + // `APIGetResponse`: a browser has no use for any of it. + repeated FileState file_states = 5; + repeated MethodHashes method_hashes = 6; + + // What produced all of the above. A dashboard whose analysis is not + // the one that wrote this cannot trust it, however untouched the + // developer's source is, so it reads everything again. + string analyzer_version = 7; } message APIGetRequest {} @@ -166,10 +204,23 @@ message APIUpdateResponse {} message APIUpdateCallsRequest { repeated MethodCalls method_calls = 1; string error = 2; + repeated FileState file_states = 3; + repeated MethodHashes method_hashes = 4; + string analyzer_version = 5; } message APIUpdateCallsResponse {} +message APIAnalysisRequest {} + +message APIAnalysisResponse { + repeated MethodCalls method_calls = 1; + string error = 2; + repeated FileState file_states = 3; + repeated MethodHashes method_hashes = 4; + string analyzer_version = 5; +} + message APIWatchRequest {} message APIWatchResponse {} @@ -251,6 +302,15 @@ service APIMethods { }; } + // What the last analysis read and concluded, which is what lets a + // dashboard starting again pick up where the one before it left + // off. Apart from `Get` because this is for the analysis to read, + // not for a browser. + rpc Analysis(APIAnalysisRequest) returns (APIAnalysisResponse) { + option (rbt.v1alpha1.method).reader = { + }; + } + // Watches the developer's API files for as long as the dashboard // application runs, reading each one that changes. rpc Watch(APIWatchRequest) returns (APIWatchResponse) { diff --git a/reboot/dashboard/servicer_reader.py b/reboot/dashboard/servicer_reader.py index 61ad5837..865a557d 100644 --- a/reboot/dashboard/servicer_reader.py +++ b/reboot/dashboard/servicer_reader.py @@ -1,12 +1,16 @@ """Describes what the developer's methods call. -Reads every source file in a directory and analyzes the whole tree at -once, because a call crosses files: a method may reach a state through -a helper written somewhere else entirely. What keeps that affordable is -that only methods whose code actually changed are analyzed again; see -`reboot.dashboard.call_analysis`. +Reads a directory of source files and analyzes the whole tree at once, +because a call crosses files: a method may reach a state through a +helper written somewhere else entirely. + +What keeps that affordable is that each pass keeps what the one before +it worked out. A file whose modified time and size are unchanged is +not read or parsed again, and a method is analyzed again only when its +own code, or the code of something it calls, has changed. """ import os +from dataclasses import dataclass, field from pathlib import Path from rbt.dashboard.v1.dashboard_pb2 import MethodCalls from reboot.dashboard.call_analysis import ( @@ -27,6 +31,39 @@ GENERATED_SUFFIXES = ('_rbt.py', '_pb2.py', '_pb2_grpc.py') +@dataclass(frozen=True) +class FileState: + """One file as a pass last saw it. + + Compared for being different rather than for being newer: checking + out an older branch moves a file's time backwards, and that is as + much an edit as any other. Size comes along because two edits a + moment apart can land on one timestamp. + """ + modified_ns: int + size: int + + +@dataclass +class Sources: + """What one pass leaves for the next. + + Every module stays, whether it was parsed this pass or kept from a + previous one, because what a method depends on may live in a file + nobody has touched: dropping those would make every method that + calls into them look changed, and the keeping would undo itself. + + `modules` is the one part a dashboard cannot leave for its + successor -- a parsed file is not something to write down -- which + is why a pass that finds every file as it was left skips itself + entirely rather than parsing them all again to learn nothing. + """ + files: dict[str, FileState] = field(default_factory=dict) + modules: dict[str, Module] = field(default_factory=dict) + analyses: dict[str, Analysis] = field(default_factory=dict) + error: str = '' + + def _files(source_directory: Path) -> list[str]: """Every file the developer wrote, relative to the directory.""" return sorted( @@ -36,16 +73,37 @@ def _files(source_directory: Path) -> list[str]: ) +def _states(directory: Path) -> dict[str, FileState]: + """Every file as it is right now. + + A file that cannot be stat'ed is left out, so that it reads as a + change and the pass that follows reports why it could not be read. + """ + states: dict[str, FileState] = {} + + for filename in _files(directory): + try: + stat = (directory / filename).stat() + except OSError: + continue + states[filename] = FileState( + modified_ns=stat.st_mtime_ns, + size=stat.st_size, + ) + + return states + + def read( source_directory: str, - cache: Optional[dict[str, Analysis]] = None, -) -> tuple[dict[str, Analysis], str]: + sources: Optional[Sources] = None, +) -> tuple[Sources, str]: """Analyzes the methods in one directory of source files. - Returns what each method calls, and a message naming the files that - could not be read. A half-written file is the normal case while - somebody is typing, so the files that did parse are still analyzed - and the rest are reported. + Returns what to hand the next pass, and a message naming the files + that could not be read. A half-written file is the normal case + while somebody is typing, so the files that did parse are still + analyzed and the rest are reported. """ directory = Path(source_directory).resolve() @@ -53,16 +111,35 @@ def read( # Worth saying rather than showing an application that calls # nothing: a directory that is not there is a typo, or one that # has yet to be made. - return {}, f'{source_directory}: no such directory' + error = f'{source_directory}: no such directory' + return Sources(error=error), error + + states = _states(directory) + if sources is not None and states == sources.files: + # Every file is as the last pass left it, so what that pass + # concluded is what this one would conclude. Said before + # anything is read, which is what makes a dashboard that + # starts again against an untouched tree cost nothing. + return sources, sources.error + + kept = sources or Sources() + + files: dict[str, FileState] = {} modules: dict[str, Module] = {} errors: list[str] = [] - for filename in _files(directory): + for filename, state in states.items(): name = module_name(filename) + if kept.files.get(filename) == state and name in kept.modules: + files[filename] = state + modules[name] = kept.modules[name] + continue + try: modules[name] = parse(name, (directory / filename).read_text()) + files[filename] = state except SyntaxError as e: errors.append( f'{os.path.join(source_directory, filename)}: ' @@ -73,7 +150,21 @@ def read( f'{os.path.join(source_directory, filename)}: {e.strerror}' ) - return analyze(modules, cache), '\n'.join(errors) + # Files that could not be stat'ed at all are left out of `states`, + # so they are named here rather than passing silently. + for filename in sorted(set(_files(directory)) - set(states)): + errors.append( + f'{os.path.join(source_directory, filename)}: could not be read' + ) + + error = '\n'.join(errors) + + return Sources( + files=files, + modules=modules, + analyses=analyze(modules, kept.analyses), + error=error, + ), error def method_calls(analyses: dict[str, Analysis]) -> list[MethodCalls]: diff --git a/reboot/dashboard/servicer_watcher.py b/reboot/dashboard/servicer_watcher.py index f092ed03..047d00bc 100644 --- a/reboot/dashboard/servicer_watcher.py +++ b/reboot/dashboard/servicer_watcher.py @@ -4,26 +4,106 @@ directory rather than the API directory. It differs in reading the whole tree on every change rather than only the files that changed, because a call crosses files: editing a helper changes what its -callers call. `reboot.dashboard.call_analysis` is what keeps that -cheap, by analyzing again only the methods whose code actually -changed. +callers call. + +What a pass works out is written down beside the answer, so a +dashboard started again reads only what has changed since the last one +ran. """ from pathlib import Path +from rbt.dashboard.v1.dashboard_pb2 import APIAnalysisResponse +from rbt.dashboard.v1.dashboard_pb2 import FileState as FileStateMessage +from rbt.dashboard.v1.dashboard_pb2 import MethodCalls, MethodHashes from rbt.dashboard.v1.dashboard_rbt import API from reboot.aio.contexts import WorkflowContext from reboot.cli.common.watch import file_watcher -from reboot.dashboard.call_analysis import Analysis +from reboot.dashboard.call_analysis import VERSION, Analysis, method_key from reboot.dashboard.constants import API_ID -from reboot.dashboard.servicer_reader import SOURCE_GLOB, method_calls, read -from typing import Optional +from reboot.dashboard.servicer_reader import ( + SOURCE_GLOB, + FileState, + Sources, + method_calls, + read, +) + + +def _restored(response: APIAnalysisResponse) -> Sources: + """What the last dashboard worked out, if this one can trust it. + + Only when the same analysis produced it: one that has since changed + would reach different conclusions from the very same files, and + nothing on disk would say so. + """ + if VERSION == '' or response.analyzer_version != VERSION: + return Sources() + + calls = { + method_key(one.state_type, one.method): one + for one in response.method_calls + } + + analyses: dict[str, Analysis] = {} + + for one in response.method_hashes: + key = method_key(one.state_type, one.method) + analyses[key] = Analysis( + # A method that calls nothing is not written down among + # the calls, so the empty result is what it had. + method_calls=calls.get( + key, + MethodCalls(state_type=one.state_type, method=one.method), + ), + hashes=dict(one.hashes), + ) + + return Sources( + files={ + state.filename: + FileState( + modified_ns=state.modified_ns, + size=state.size, + ) for state in response.file_states + }, + analyses=analyses, + error=response.error, + ) + + +def _file_states(sources: Sources) -> list[FileStateMessage]: + return [ + FileStateMessage( + filename=filename, + modified_ns=state.modified_ns, + size=state.size, + ) for filename, state in sorted(sources.files.items()) + ] + + +def _method_hashes(sources: Sources) -> list[MethodHashes]: + return [ + MethodHashes( + state_type=sources.analyses[key].method_calls.state_type, + method=sources.analyses[key].method_calls.method, + hashes=sources.analyses[key].hashes, + ) for key in sorted(sources.analyses) + ] async def watch(context: WorkflowContext, *, source_directory: str) -> None: """Updates what the methods call, for as long as this runs.""" directory = Path(source_directory).resolve() - analyses: dict[str, Analysis] = {} - updated: Optional[tuple] = None + response = await API.ref(API_ID).Analysis(context) + + sources = _restored(response) + + # What is already written, so that a dashboard starting against a + # tree nobody has touched writes nothing at all. + updated = ( + (list(response.method_calls), + response.error) if len(sources.files) > 0 else None + ) with file_watcher() as watcher: async for iteration in context.loop('analyze what changed'): @@ -36,9 +116,9 @@ async def watch(context: WorkflowContext, *, source_directory: str) -> None: [SOURCE_GLOB], root_dir=str(directory), ) as event: - analyses, error = read(source_directory, analyses) + sources, error = read(source_directory, sources) - current = (method_calls(analyses), error) + current = (method_calls(sources.analyses), error) if current != updated: updated = current @@ -48,6 +128,9 @@ async def watch(context: WorkflowContext, *, source_directory: str) -> None: context, method_calls=current[0], error=current[1], + file_states=_file_states(sources), + method_hashes=_method_hashes(sources), + analyzer_version=VERSION, ) await event diff --git a/reboot/dashboard/servicers.py b/reboot/dashboard/servicers.py index 0092e764..33bef4fd 100644 --- a/reboot/dashboard/servicers.py +++ b/reboot/dashboard/servicers.py @@ -2,6 +2,8 @@ import os import reboot.std.presence.v1.presence from rbt.dashboard.v1.dashboard_pb2 import ( + APIAnalysisRequest, + APIAnalysisResponse, APIGetRequest, APIGetResponse, APIUpdateCallsRequest, @@ -93,6 +95,19 @@ async def Update( self.state.error = request.error return APIUpdateResponse() + async def Analysis( + self, + context: ReaderContext, + request: APIAnalysisRequest, + ) -> APIAnalysisResponse: + return APIAnalysisResponse( + method_calls=self.state.method_calls, + error=self.state.calls_error, + file_states=self.state.file_states, + method_hashes=self.state.method_hashes, + analyzer_version=self.state.analyzer_version, + ) + async def UpdateCalls( self, context: WriterContext, @@ -101,6 +116,15 @@ async def UpdateCalls( del self.state.method_calls[:] self.state.method_calls.extend(request.method_calls) self.state.calls_error = request.error + + del self.state.file_states[:] + self.state.file_states.extend(request.file_states) + + del self.state.method_hashes[:] + self.state.method_hashes.extend(request.method_hashes) + + self.state.analyzer_version = request.analyzer_version + return APIUpdateCallsResponse() diff --git a/tests/reboot/dashboard/servicer_reader_tests.py b/tests/reboot/dashboard/servicer_reader_tests.py index 66535aa5..bd8e0fee 100644 --- a/tests/reboot/dashboard/servicer_reader_tests.py +++ b/tests/reboot/dashboard/servicer_reader_tests.py @@ -3,11 +3,15 @@ Nothing is imported and nothing is generated: `rbt generate` has not run, there is no application, and the files are read as they are. """ +import os import tempfile import unittest from pathlib import Path from rbt.dashboard.v1.dashboard_pb2 import Call +from reboot.dashboard import servicer_reader from reboot.dashboard.servicer_reader import method_calls, read +from typing import Optional +from unittest.mock import patch SERVICER = ''' from bank.v1.account_rbt import Account @@ -45,18 +49,28 @@ def setUp(self) -> None: def tearDown(self) -> None: self._directory.cleanup() - def _write(self, name: str, source: str) -> None: - (self.directory / name).write_text(source) + def _write( + self, + name: str, + source: str, + modified_ns: Optional[int] = None, + ) -> None: + path = self.directory / name + path.write_text(source) + if modified_ns is not None: + # Said outright, so that what a timestamp catches is not + # left to how fast the test happens to run. + os.utime(path, ns=(modified_ns, modified_ns)) def test_describes_what_a_method_calls(self) -> None: self._write('account_servicer.py', SERVICER) self._write('helpers.py', HELPERS) - analyses, error = read(str(self.directory)) + sources, error = read(str(self.directory)) self.assertEqual(error, '') - described = method_calls(analyses) + described = method_calls(sources.analyses) self.assertEqual( [(one.state_type, one.method) for one in described], @@ -76,11 +90,11 @@ def test_a_method_that_calls_nothing_is_left_out(self) -> None: self._write('account_servicer.py', SERVICER) self._write('helpers.py', HELPERS) - analyses, _ = read(str(self.directory)) + sources, _ = read(str(self.directory)) self.assertNotIn( 'quiet', - [one.method for one in method_calls(analyses)], + [one.method for one in method_calls(sources.analyses)], ) def test_generated_files_are_not_read(self) -> None: @@ -99,20 +113,114 @@ def test_a_half_written_file_is_reported_and_the_rest_still_read( self._write('helpers.py', HELPERS) self._write('halfway.py', 'async def transfer(context,') - analyses, error = read(str(self.directory)) + sources, error = read(str(self.directory)) self.assertIn('halfway.py', error) self.assertEqual( - [one.method for one in method_calls(analyses)], + [one.method for one in method_calls(sources.analyses)], ['move'], ) def test_a_directory_that_is_not_there_is_said_so(self) -> None: - analyses, error = read(str(self.directory / 'nowhere')) + sources, error = read(str(self.directory / 'nowhere')) - self.assertEqual(analyses, {}) + self.assertEqual(sources.analyses, {}) self.assertIn('no such directory', error) + ################################################################### + # Reading again only what changed. + + def _counting_parse(self): + """Records the name of every file actually parsed.""" + parsed: list[str] = [] + real = servicer_reader.parse + + def counted(name: str, source: str): + parsed.append(name) + return real(name, source) + + return parsed, patch.object(servicer_reader, 'parse', counted) + + def test_an_untouched_tree_is_not_read_again(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + + first, _ = read(str(self.directory)) + + parsed, counting = self._counting_parse() + with counting: + second, _ = read(str(self.directory), first) + + self.assertEqual(parsed, []) + self.assertEqual( + method_calls(second.analyses), method_calls(first.analyses) + ) + + def test_only_the_file_that_changed_is_read_again(self) -> None: + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS) + + first, _ = read(str(self.directory)) + + self._write('helpers.py', HELPERS.replace('withdraw', 'pay')) + + parsed, counting = self._counting_parse() + with counting: + second, _ = read(str(self.directory), first) + + self.assertEqual(parsed, ['helpers']) + self.assertEqual( + [call.method for call in method_calls(second.analyses)[0].calls], + ['pay'], + ) + + def test_an_edit_of_the_same_size_is_noticed(self) -> None: + """Two file names of a length are what a timestamp is for.""" + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS, modified_ns=1_000_000_000) + + first, _ = read(str(self.directory)) + + self._write( + 'helpers.py', + HELPERS.replace('withdraw', 'deposit'), + modified_ns=2_000_000_000, + ) + + parsed, counting = self._counting_parse() + with counting: + second, _ = read(str(self.directory), first) + + self.assertEqual(parsed, ['helpers']) + self.assertEqual( + [call.method for call in method_calls(second.analyses)[0].calls], + ['deposit'], + ) + + def test_a_file_whose_time_went_backwards_is_noticed(self) -> None: + """Checking out an older branch moves a file's time backwards, + which is as much an edit as any other.""" + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS, modified_ns=2_000_000_000) + + first, _ = read(str(self.directory)) + + self._write( + 'helpers.py', + HELPERS.replace('withdraw', 'deposit'), + modified_ns=1_000_000_000, + ) + + parsed, counting = self._counting_parse() + with counting: + second, _ = read(str(self.directory), first) + + self.assertEqual(parsed, ['helpers']) + self.assertEqual( + [call.method for call in method_calls(second.analyses)[0].calls], + ['deposit'], + ) + def test_reading_twice_says_the_same_thing(self) -> None: self._write('account_servicer.py', SERVICER) self._write('helpers.py', HELPERS) @@ -120,7 +228,9 @@ def test_reading_twice_says_the_same_thing(self) -> None: first, _ = read(str(self.directory)) second, _ = read(str(self.directory), first) - self.assertEqual(method_calls(first), method_calls(second)) + self.assertEqual( + method_calls(first.analyses), method_calls(second.analyses) + ) if __name__ == '__main__': diff --git a/tests/reboot/dashboard/servicer_watcher_tests.py b/tests/reboot/dashboard/servicer_watcher_tests.py index 3cc5e4a2..f4d5be32 100644 --- a/tests/reboot/dashboard/servicer_watcher_tests.py +++ b/tests/reboot/dashboard/servicer_watcher_tests.py @@ -10,12 +10,16 @@ from pathlib import Path from rbt.dashboard.v1.dashboard_rbt import API from reboot.aio.tests import Reboot +from reboot.dashboard import servicer_reader +from reboot.dashboard.call_analysis import VERSION from reboot.dashboard.constants import ( API_ID, ENVVAR_RBT_API_DIRECTORY, ENVVAR_RBT_SOURCE_DIRECTORY, ) from reboot.dashboard.main import application +from reboot.dashboard.servicer_reader import Sources, method_calls, read +from reboot.dashboard.servicer_watcher import _restored from typing import Optional from unittest.mock import patch @@ -115,6 +119,63 @@ async def test_calls_follow_the_source(self) -> None: ['deposit'], ) + async def test_a_dashboard_starting_again_reads_nothing(self) -> None: + """What one dashboard worked out is written down beside the + answer, so the next one against an untouched tree has nothing + to read, parse or analyze.""" + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS.format(method='withdraw')) + + await self._wait_for(lambda api: len(api.method_calls) == 1) + + # What a dashboard starting again would find, taken from the + # state the running one wrote. + context = self.rbt.create_external_context(name=self.id()) + analysis = await API.ref(API_ID).Analysis(context) + + self.assertEqual(analysis.analyzer_version, VERSION) + self.assertEqual( + sorted(state.filename for state in analysis.file_states), + ['account_servicer.py', 'helpers.py'], + ) + + restored = _restored(analysis) + + parsed: list[str] = [] + real = servicer_reader.parse + + def counted(name: str, source: str): + parsed.append(name) + return real(name, source) + + with patch.object(servicer_reader, 'parse', counted): + sources, error = read(str(self.source), restored) + + self.assertEqual(parsed, []) + self.assertEqual(error, '') + self.assertEqual( + [ + (one.state_type, one.method, one.calls[0].method) + for one in method_calls(sources.analyses) + ], + [('bank.v1.Account', 'move', 'withdraw')], + ) + + async def test_an_analysis_that_has_changed_is_not_trusted(self) -> None: + """Results are only worth keeping if the analysis that wrote + them is the one about to use them.""" + self._write('account_servicer.py', SERVICER) + self._write('helpers.py', HELPERS.format(method='withdraw')) + + await self._wait_for(lambda api: len(api.method_calls) == 1) + + context = self.rbt.create_external_context(name=self.id()) + analysis = await API.ref(API_ID).Analysis(context) + + analysis.analyzer_version = 'some other analysis' + + self.assertEqual(_restored(analysis), Sources()) + async def test_a_half_written_file_is_reported(self) -> None: self._write('account_servicer.py', SERVICER) self._write('helpers.py', HELPERS.format(method='withdraw'))