From 87f3b2caef373272500f3596145fa721c40f08c7 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Thu, 13 Aug 2026 22:14:06 +0000 Subject: [PATCH 1/9] 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 186a98650c10309b257e35487203a4f7720e0d28 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 11 Aug 2026 03:23:51 +0000 Subject: [PATCH 2/9] 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 d3710567..629cbfbd 100644 --- a/reboot/templates/reboot.py.j2 +++ b/reboot/templates/reboot.py.j2 @@ -2697,7 +2697,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 }}'. @@ -2746,7 +2746,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 8eb0ed90fa68fd57d3fb42985760dadcb7ba1533 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 11 Aug 2026 03:25:02 +0000 Subject: [PATCH 3/9] 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 36cb45756c023cc2c9f67a03857606bd1b637312 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Wed, 12 Aug 2026 04:50:45 +0000 Subject: [PATCH 4/9] 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 374e23d7499eb4a384493c9d72b9c68c6f4dc70a Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Thu, 13 Aug 2026 06:23:03 +0000 Subject: [PATCH 5/9] 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 8bf10e89728a2d4d9af89e3ffe6e6422907b56cf Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Mon, 17 Aug 2026 22:58:11 +0000 Subject: [PATCH 6/9] @benh PR feedback Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 1 + pnpm-lock.yaml | 172 +++++++++++++++-- rbt/dashboard/v1/BUILD.bazel | 3 + rbt/dashboard/v1/dashboard.proto | 58 ++---- reboot/cli/commands/dashboard.py | 17 +- reboot/dashboard/BUILD.bazel | 14 ++ reboot/dashboard/api_reader.py | 153 +++++++-------- reboot/dashboard/api_watcher.py | 36 ++-- reboot/dashboard/frontend/BUILD.bazel | 6 + reboot/dashboard/frontend/dashboard.css | 73 ++++--- reboot/dashboard/frontend/src/description.ts | 143 ++++++++++++++ reboot/dashboard/frontend/src/main.tsx | 179 +++++++++++++----- reboot/dashboard/frontend/tsconfig.json | 2 + reboot/dashboard/servicers.py | 18 +- .../chat-app/references/api-method-types.md | 10 + .../skills/python/references/api-methods.md | 29 +++ .../skills/python/references/api-pydantic.md | 26 ++- .../method-description-out-of-mcp-options.md | 78 ++++++++ tests/reboot/cli/dashboard_tests.py | 10 + tests/reboot/dashboard/BUILD.bazel | 55 ++++++ tests/reboot/dashboard/api/shop/v1/shop.py | 14 ++ tests/reboot/dashboard/api_reader_tests.py | 87 +++++++-- tests/reboot/dashboard/api_watcher_tests.py | 18 +- tests/reboot/dashboard/dashboard_tests.py | 85 ++++++--- tests/reboot/dashboard/description.test.ts | 55 ++++++ tests/reboot/dashboard/package.json | 3 + tests/reboot/dashboard/vitest.config.mjs | 6 + 27 files changed, 1082 insertions(+), 269 deletions(-) create mode 100644 reboot/dashboard/frontend/src/description.ts create mode 100644 reboot/plugin/skills/upgrade/migrations/next/method-description-out-of-mcp-options.md create mode 100644 tests/reboot/dashboard/description.test.ts create mode 100644 tests/reboot/dashboard/package.json create mode 100644 tests/reboot/dashboard/vitest.config.mjs diff --git a/package.json b/package.json index 6ce1781e..9d5edbb1 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "@reboot-dev/reboot-std": "1.4.1", "@reboot-dev/reboot-web": "1.4.1", "@reboot-dev/reboot": "1.4.1", + "@stoplight/json-schema-tree": "4.0.0", "@modelcontextprotocol/ext-apps": "1.5.0", "@modelcontextprotocol/sdk": "1.29.0", "@bufbuild/protobuf": "1.10.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 247e10c5..471970c6 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -24,29 +24,32 @@ importers: specifier: 1.29.0 version: 1.29.0(zod@3.25.76) '@reboot-dev/reboot': - specifier: 1.2.1 + specifier: 1.4.1 version: link:reboot/nodejs '@reboot-dev/reboot-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:rbt/v1alpha1 '@reboot-dev/reboot-react': - specifier: 1.2.1 + specifier: 1.4.1 version: link:reboot/react '@reboot-dev/reboot-std': - specifier: 1.2.1 + specifier: 1.4.1 version: link:reboot/std '@reboot-dev/reboot-std-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:rbt/std '@reboot-dev/reboot-std-react': - specifier: 1.2.1 + specifier: 1.4.1 version: link:reboot/std/react '@reboot-dev/reboot-web': - specifier: 1.2.1 + specifier: 1.4.1 version: link:reboot/web '@standard-schema/spec': specifier: 1.0.0 version: 1.0.0 + '@stoplight/json-schema-tree': + specifier: 4.0.0 + version: 4.0.0 '@testing-library/react': specifier: ^16.3.0 version: 16.3.2(@testing-library/dom@10.4.1)(@types/react@19.2.7)(react-dom@19.2.1)(react@19.2.1) @@ -187,7 +190,7 @@ importers: specifier: 1.10.1 version: 1.10.1 '@reboot-dev/reboot-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../rbt/v1alpha1 '@scarf/scarf': specifier: 1.4.0 @@ -254,10 +257,10 @@ importers: specifier: 1.29.0 version: 1.29.0(zod@3.25.76) '@reboot-dev/reboot-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../rbt/v1alpha1 '@reboot-dev/reboot-web': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../web '@scarf/scarf': specifier: 1.4.0 @@ -300,10 +303,10 @@ importers: reboot/std: dependencies: '@reboot-dev/reboot': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../nodejs '@reboot-dev/reboot-std-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../rbt/std '@scarf/scarf': specifier: 1.4.0 @@ -316,16 +319,16 @@ importers: reboot/std/react: dependencies: '@reboot-dev/reboot-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../../rbt/v1alpha1 '@reboot-dev/reboot-react': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../react '@reboot-dev/reboot-std-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../../rbt/std '@reboot-dev/reboot-web': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../web '@scarf/scarf': specifier: 1.4.0 @@ -341,7 +344,7 @@ importers: specifier: 1.10.1 version: 1.10.1 '@reboot-dev/reboot-api': - specifier: 1.2.1 + specifier: 1.4.1 version: link:../../rbt/v1alpha1 '@scarf/scarf': specifier: 1.4.0 @@ -1634,6 +1637,63 @@ packages: resolution: {integrity: sha512-m2bOd0f2RT9k8QJx1JN85cZYyH1RqFBdlwtkSlf4tBDYLCiiZnv1fIIwacK6cqwXavOydf0NPToMQgpKq+dVlA==} dev: false + /@stoplight/json-schema-merge-allof@0.8.0: + resolution: {integrity: sha512-g8e0s43v96Xbzvd8d6KKUuJTO16CS2oJglJrviUi8ASIUxzFvAJqTHWLtGmpTryisQopqg1evXGJfi0+164+Qw==} + dependencies: + compute-lcm: 1.1.2 + json-schema-compare: 0.2.2 + lodash: 4.18.1 + dev: false + + /@stoplight/json-schema-tree@4.0.0: + resolution: {integrity: sha512-SAGtof+ihIdPqETR+7XXOaqZJcrbSih/xEahaw5t1nXk5sVW6ss2l5A1WCIuvtvnQiUKnBfanmZU4eoM1ZvItg==} + engines: {node: '>=10.18'} + dependencies: + '@stoplight/json': 3.21.7 + '@stoplight/json-schema-merge-allof': 0.8.0 + '@stoplight/lifecycle': 2.3.3 + '@types/json-schema': 7.0.15 + magic-error: 0.0.1 + dev: false + + /@stoplight/json@3.21.7: + resolution: {integrity: sha512-xcJXgKFqv/uCEgtGlPxy3tPA+4I+ZI4vAuMJ885+ThkTHFVkC+0Fm58lA9NlsyjnkpxFh4YiQWpH+KefHdbA0A==} + engines: {node: '>=8.3.0'} + dependencies: + '@stoplight/ordered-object-literal': 1.0.5 + '@stoplight/path': 1.3.2 + '@stoplight/types': 13.20.0 + jsonc-parser: 2.2.1 + lodash: 4.18.1 + safe-stable-stringify: 1.1.1 + dev: false + + /@stoplight/lifecycle@2.3.3: + resolution: {integrity: sha512-JbPRTIzPZabeYPAk5+gdsnfwAxqW35G9e0ZjOG3toUmNViLOsEzuK4vpWd+Prv2Mw8HRmu+haiYizteZp6mk0w==} + engines: {node: '>=8.3.0'} + dependencies: + tslib: 2.7.0 + wolfy87-eventemitter: 5.2.9 + dev: false + + /@stoplight/ordered-object-literal@1.0.5: + resolution: {integrity: sha512-COTiuCU5bgMUtbIFBuyyh2/yVVzlr5Om0v5utQDgBCuQUOPgU1DwoffkTfg4UBQOvByi5foF4w4T+H9CoRe5wg==} + engines: {node: '>=8'} + dev: false + + /@stoplight/path@1.3.2: + resolution: {integrity: sha512-lyIc6JUlUA8Ve5ELywPC8I2Sdnh1zc1zmbYgVarhXIp9YeAB0ReeqmGEOWNtlHkbP2DAA1AL65Wfn2ncjK/jtQ==} + engines: {node: '>=8'} + dev: false + + /@stoplight/types@13.20.0: + resolution: {integrity: sha512-2FNTv05If7ib79VPDA/r9eUet76jewXFH2y2K5vuge6SXbRHtWBhcaRmu+6QpF4/WRNoJj5XYRSwLGXDxysBGA==} + engines: {node: ^12.20 || >=14.13} + dependencies: + '@types/json-schema': 7.0.15 + utility-types: 3.11.0 + dev: false + /@testing-library/dom@10.4.1: resolution: {integrity: sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==} engines: {node: '>=18'} @@ -1747,6 +1807,10 @@ packages: /@types/http-errors@2.0.5: resolution: {integrity: sha512-r8Tayk8HJnX0FztbZN7oVqGccWgw98T/0neJphO91KkmOzug1KkofZURD4UaD5uH8AqcFLfdPErnBod0u71/qg==} + /@types/json-schema@7.0.15: + resolution: {integrity: sha512-5+fP8P8MFNC+AyZCDxrB2pkZFPGzqQWUzpSeuuVLvm8VMcorNYavBqoFcxK8bQz4Qsbn4oUEEem4wDLfcysGHA==} + dev: false + /@types/node@20.11.5: resolution: {integrity: sha512-g557vgQjUUfN76MZAN/dt1z3dzcUsimuysco0KeluHgrPdJXkP/XdAURgyO2W9fZWHRtRBiVKzKn8vyOAwlG+w==} dependencies: @@ -2121,6 +2185,23 @@ packages: resolution: {integrity: sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA==} dev: false + /compute-gcd@1.2.1: + resolution: {integrity: sha512-TwMbxBNz0l71+8Sc4czv13h4kEqnchV9igQZBi6QUaz09dnz13juGnnaWWJTRsP3brxOoxeB4SA2WELLw1hCtg==} + dependencies: + validate.io-array: 1.0.6 + validate.io-function: 1.0.2 + validate.io-integer-array: 1.0.0 + dev: false + + /compute-lcm@1.1.2: + resolution: {integrity: sha512-OFNPdQAXnQhDSKioX8/XYT6sdUlXwpeMjfd6ApxMJfyZ4GxmLR1xvMERctlYhlHwIiz6CSpBc2+qYKjHGZw4TQ==} + dependencies: + compute-gcd: 1.2.1 + validate.io-array: 1.0.6 + validate.io-function: 1.0.2 + validate.io-integer-array: 1.0.0 + dev: false + /content-disposition@1.0.1: resolution: {integrity: sha512-oIXISMynqSqm241k6kcQ5UwttDILMK4BiurCfGEREw6+X9jkkpEe5T9FZaApyLGGOnFuyMWZpdolTXMtvEJ08Q==} engines: {node: '>=18'} @@ -2926,6 +3007,12 @@ packages: hasBin: true dev: false + /json-schema-compare@0.2.2: + resolution: {integrity: sha512-c4WYmDKyJXhs7WWvAWm3uIYnfyWFoIp+JEoX34rctVvEkMYCPGhXtvmFFXiffBbxfZsvQ0RNnV5H7GvDF5HCqQ==} + dependencies: + lodash: 4.18.1 + dev: false + /json-schema-traverse@1.0.0: resolution: {integrity: sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug==} dev: false @@ -2940,6 +3027,10 @@ packages: hasBin: true dev: false + /jsonc-parser@2.2.1: + resolution: {integrity: sha512-o6/yDBYccGvTz1+QFevz6l6OBZ2+fMVu2JZ9CIhzsYRX4mjaK5IyX9eldUdCmga16zlgQxyrj5pt9kzuj2C02w==} + dev: false + /lodash.curry@4.1.1: resolution: {integrity: sha512-/u14pXGviLaweY5JI0IUzgzF2J6Ne8INyzAZjImcryjgkZ+ebruBxy2/JaOOkTqScddcYtakjhSaeemV8lR0tA==} dev: false @@ -2948,6 +3039,10 @@ packages: resolution: {integrity: sha512-ff3BX/tSioo+XojX4MOsOMhJw0nZoUEF011LX8g8d3gvjVbxd89cCio4BCXronjxcTUIJUoqKEUA+n4CqvvRPw==} dev: false + /lodash@4.18.1: + resolution: {integrity: sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==} + dev: false + /loose-envify@1.4.0: resolution: {integrity: sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==} hasBin: true @@ -2983,6 +3078,11 @@ packages: hasBin: true dev: false + /magic-error@0.0.1: + resolution: {integrity: sha512-1+N1ET8cbC5bfLQZcRojClzgK2gbUt9keTMr9OJeuXnQKWsfwRRRICuMA3HKaCIXFEgKzxivuMGCNKD7cdU5pg==} + engines: {node: '>=10'} + dev: false + /magic-string@0.30.17: resolution: {integrity: sha512-sNPKHvyjVf7gyjwS4xGTaW/mCnF8wnjtifKBEhxfZ7E/S8tQ0rssrwGNn6q8JH/ohItJfSQp9mBtQYuTlH5QnA==} dependencies: @@ -3483,6 +3583,10 @@ packages: resolution: {integrity: sha512-guoltQEx+9aMf2gDZ0s62EcV8lsXR+0w8915TC3ITdn2YueuNjdAYh/levpU9nFaoChh9RUS5ZdQMrKfVEN9tw==} dev: false + /safe-stable-stringify@1.1.1: + resolution: {integrity: sha512-ERq4hUjKDbJfE4+XtZLFPCDi8Vb1JqaxAPTxWFLBx8XcAlf9Bda/ZJdVezs/NAfsMQScyIlUMx+Yeu7P7rx5jw==} + dev: false + /safer-buffer@2.1.2: resolution: {integrity: sha512-YZo3K82SD7Riyi0E1EQPojLz7kpepnSQI9IyPbHHg1XXXevb5dJI7tpyN2ADxGcQbHG7vcyRHk0cbwqcQriUtg==} dev: false @@ -3925,11 +4029,41 @@ packages: use-isomorphic-layout-effect: 1.2.1(@types/react@19.2.7)(react@19.2.1) dev: false + /utility-types@3.11.0: + resolution: {integrity: sha512-6Z7Ma2aVEWisaL6TvBCy7P8rm2LQoPv6dJ7ecIaIixHcwfbJ0x7mWdbcwlIM5IGQxPZSFYeqRCqlOOeKoJYMkw==} + engines: {node: '>= 4'} + dev: false + /uuid@11.1.1: resolution: {integrity: sha512-vIYxrBCC/N/K+Js3qSN88go7kIfNPssr/hHCesKCQNAjmgvYS2oqr69kIufEG+O4+PfezOH4EbIeHCfFov8ZgQ==} hasBin: true dev: false + /validate.io-array@1.0.6: + resolution: {integrity: sha512-DeOy7CnPEziggrOO5CZhVKJw6S3Yi7e9e65R1Nl/RTN1vTQKnzjfvks0/8kQ40FP/dsjRAOd4hxmJ7uLa6vxkg==} + dev: false + + /validate.io-function@1.0.2: + resolution: {integrity: sha512-LlFybRJEriSuBnUhQyG5bwglhh50EpTL2ul23MPIuR1odjO7XaMLFV8vHGwp7AZciFxtYOeiSCT5st+XSPONiQ==} + dev: false + + /validate.io-integer-array@1.0.0: + resolution: {integrity: sha512-mTrMk/1ytQHtCY0oNO3dztafHYyGU88KL+jRxWuzfOmQb+4qqnWmI+gykvGp8usKZOM0H7keJHEbRaFiYA0VrA==} + dependencies: + validate.io-array: 1.0.6 + validate.io-integer: 1.0.5 + dev: false + + /validate.io-integer@1.0.5: + resolution: {integrity: sha512-22izsYSLojN/P6bppBqhgUDjCkr5RY2jd+N2a3DCAUey8ydvrZ/OkGvFPR7qfOpwR2LC5p4Ngzxz36g5Vgr/hQ==} + dependencies: + validate.io-number: 1.0.3 + dev: false + + /validate.io-number@1.0.3: + resolution: {integrity: sha512-kRAyotcbNaSYoDnXvb4MHg/0a1egJdLwS6oJ38TJY7aw9n93Fl/3blIXdyYvPOp55CNxywooG/3BcrwNrBpcSg==} + dev: false + /vary@1.1.2: resolution: {integrity: sha512-BNGbWLfd0eUPabhkXUVm0j8uuvREyTh5ovRa/dyow/BqAbZJyC+5fU+IzQOzmAKzYqYRAISoRhdQr3eIZ/PXqg==} engines: {node: '>= 0.8'} @@ -4154,6 +4288,10 @@ packages: stackback: 0.0.2 dev: false + /wolfy87-eventemitter@5.2.9: + resolution: {integrity: sha512-P+6vtWyuDw+MB01X7UeF8TaHBvbCovf4HPEMF/SV7BdDc1SMTiBy13SRD71lQh4ExFTG1d/WNzDGDCyOKSMblw==} + dev: false + /wrap-ansi@7.0.0: resolution: {integrity: sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==} engines: {node: '>=10'} diff --git a/rbt/dashboard/v1/BUILD.bazel b/rbt/dashboard/v1/BUILD.bazel index 84478778..14e6e95f 100644 --- a/rbt/dashboard/v1/BUILD.bazel +++ b/rbt/dashboard/v1/BUILD.bazel @@ -15,6 +15,7 @@ proto_library( visibility = ["//visibility:public"], deps = [ "@com_github_reboot_dev_reboot//rbt/v1alpha1:options_proto", + "@com_google_protobuf//:struct_proto", ], ) @@ -36,6 +37,7 @@ js_proto_library( # the `proto_libraries` here. "@com_github_reboot_dev_reboot//rbt/v1alpha1:options_proto", "@com_google_protobuf//:descriptor_proto", + "@com_google_protobuf//:struct_proto", ], visibility = ["//visibility:public"], ) @@ -62,6 +64,7 @@ js_reboot_react_library( ":dashboard_proto", "@com_github_reboot_dev_reboot//rbt/v1alpha1:options_proto", "@com_google_protobuf//:descriptor_proto", + "@com_google_protobuf//:struct_proto", ], visibility = ["//visibility:public"], ) diff --git a/rbt/dashboard/v1/dashboard.proto b/rbt/dashboard/v1/dashboard.proto index 3c5a8f01..bcb2c5b5 100644 --- a/rbt/dashboard/v1/dashboard.proto +++ b/rbt/dashboard/v1/dashboard.proto @@ -2,44 +2,11 @@ syntax = "proto3"; package rbt.dashboard.v1; +import "google/protobuf/struct.proto"; 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 state type does, in the author's own words. - optional string description = 5; -} - -//////////////////////////////////////////////////////////////////////// - // What the dashboard application has read of the developer's API // files, so that a browser can read it without reaching the // application itself. @@ -48,25 +15,30 @@ message API { }; // The state types the developer's API files declare, which exist - // before the application does. - repeated StateTypeInfo state_types = 1; + // before the application does. A JSON array: each state type + // carries its methods and a `$defs` of every type they mention, + // referred to by `$ref`. `Value` because the types are Pydantic's + // own JSON Schema, which is recursive and open-ended, and describing + // it in proto would mean owning a second copy of a mapping Pydantic + // already has. + google.protobuf.Value 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; + optional string error = 2; } message APIGetRequest {} message APIGetResponse { - repeated StateTypeInfo state_types = 1; - string error = 2; + google.protobuf.Value state_types = 1; + optional string error = 2; } message APIUpdateRequest { - repeated StateTypeInfo state_types = 1; - string error = 2; + google.protobuf.Value state_types = 1; + optional string error = 2; } message APIUpdateResponse {} @@ -121,8 +93,8 @@ message PreferencesSetSuppressOpenOnRestartRequest { message PreferencesSetSuppressOpenOnRestartResponse {} message PreferencesSetExpandedRequest { - // The fully qualified name of one state type, spelled the way - // `StateTypeInfo` spells it. + // The fully qualified name of one state type, spelled the way the + // description spells it. string state_type = 1; bool expanded = 2; diff --git a/reboot/cli/commands/dashboard.py b/reboot/cli/commands/dashboard.py index 35f51ea0..b2a3577b 100644 --- a/reboot/cli/commands/dashboard.py +++ b/reboot/cli/commands/dashboard.py @@ -26,11 +26,13 @@ ) from reboot.settings import ( ENVVAR_RBT_DEV, + ENVVAR_RBT_EFFECT_VALIDATION, ENVVAR_RBT_FRONTEND_DIST_PATH, ENVVAR_RBT_FRONTEND_HOST, ENVVAR_RBT_FRONTEND_ROOT_PATH, ENVVAR_RBT_NAME, ENVVAR_RBT_NODEJS, + ENVVAR_RBT_SERVE, ENVVAR_RBT_SERVERS, ENVVAR_RBT_STATE_DIRECTORY, ENVVAR_REBOOT_CRYPTO_ROOT_KEYS, @@ -99,7 +101,16 @@ def _dashboard_env( ): composed.pop(name, None) - composed[ENVVAR_RBT_DEV] = 'true' + # Served with `rbt serve` defaults rather than `rbt dev` ones. + # Popped rather than left unset, since `detect_run_environment` + # reads `RBT_DEV` first. + composed.pop(ENVVAR_RBT_DEV, None) + composed[ENVVAR_RBT_SERVE] = 'true' + + # Also what `rbt serve` sets: `RBT_SERVE` alone is not enough to + # produce a `rbt serve` environment. + composed[ENVVAR_RBT_EFFECT_VALIDATION] = 'DISABLED' + composed[ENVVAR_REBOOT_EXPECTED_VERSION] = REBOOT_VERSION composed[ENVVAR_REBOOT_LOCAL_ENVOY] = 'true' composed[ENVVAR_REBOOT_LOCAL_ENVOY_PORT] = str(port) @@ -165,10 +176,6 @@ async def _run_dashboard( '-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 diff --git a/reboot/dashboard/BUILD.bazel b/reboot/dashboard/BUILD.bazel index 4a7a1d0c..505abb47 100644 --- a/reboot/dashboard/BUILD.bazel +++ b/reboot/dashboard/BUILD.bazel @@ -19,6 +19,20 @@ py_library( ], ) +# The reader as a command, so a `genrule` can produce a description +# for the test that holds `api_reader.py` and `description.ts` to the +# same shape. `main()` writes the description to stdout. +py_binary( + name = "api_reader", + srcs = ["api_reader.py"], + main = "api_reader.py", + visibility = ["//visibility:public"], + deps = [ + "//rbt/dashboard/v1:dashboard_py_reboot", + "//reboot:api_py", + ], +) + py_library( name = "api_watcher_py", srcs = ["api_watcher.py"], diff --git a/reboot/dashboard/api_reader.py b/reboot/dashboard/api_reader.py index dbf55816..9610030e 100644 --- a/reboot/dashboard/api_reader.py +++ b/reboot/dashboard/api_reader.py @@ -5,8 +5,8 @@ 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. +and it writes a JSON list of state types 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 @@ -14,78 +14,79 @@ 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. +Types are Pydantic's own JSON Schema rather than anything spelled +here, so a nested type is followed rather than named: a state type +carries a `$defs` of everything its methods mention, and its state, +requests, responses and errors are `$ref`s into it. Scoped per state +type, so two files declaring the same name do not collide. """ 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 pydantic.json_schema import models_json_schema 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], +from typing import Optional + + +def _schemas_of(models: list[type[Model]]) -> tuple[dict, dict]: + """The `$defs` for `models`, and a `$ref` to each by model. + + One call for the whole state type, so a type two of its methods + both mention is described once. + """ + if len(models) == 0: + return {}, {} + + # `models_json_schema` dedupes for us, but wants each model once. + unique = list(dict.fromkeys(models)) + + refs, schema = models_json_schema( + [(model, 'validation') for model in unique], + ref_template='#/$defs/{model}', ) + return schema.get('$defs', {}), { + model: refs[(model, 'validation')] for model in unique + } + + +def _models_of(type_obj) -> list[type[Model]]: + """Every model one state type mentions, state first.""" + models: list[type[Model]] = [type_obj.state] + + for spec in type_obj.methods.values(): + # A `UI` method has no RPC to call, so there is nothing to + # put in a method row for it. + if not isinstance(spec, MethodModel): + continue + if spec.request is not None: + models.append(spec.request) + if spec.response is not None: + models.append(spec.response) + models.extend(spec.errors) + + return models + + +def _describe_method(method_name: str, spec: MethodModel, refs: dict) -> dict: + method: dict = { + 'name': method_name, + 'kind': spec.kind.value, + 'factory': spec.factory, + 'mcp': spec.mcp is not None, + 'errors': [refs[error] for error in spec.errors], + } + if spec.request is not None: - info.arguments.extend(_fields_of(spec.request)) + method['request'] = refs[spec.request] if spec.response is not None: - info.returns.extend(_fields_of(spec.response)) + method['response'] = refs[spec.response] if spec.description is not None: - info.description = spec.description + method['description'] = spec.description - return info + return method def describe(api_directory: str, filename: str) -> list[dict]: @@ -118,21 +119,25 @@ def describe(api_directory: str, filename: str) -> list[dict]: 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 + definitions, refs = _schemas_of(_models_of(type_obj)) + + state_type: dict = { + 'name': f'{package}.{type_name}', + 'file': file, + 'state': refs[type_obj.state], + 'methods': + [ + _describe_method(method_name, spec, refs) + for method_name, spec in type_obj.methods.items() + if isinstance(spec, MethodModel) + ], + '$defs': definitions, + } - 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)) + if type_obj.description is not None: + state_type['description'] = type_obj.description - described.append(MessageToDict(info, preserving_proto_field_name=True)) + described.append(state_type) return described diff --git a/reboot/dashboard/api_watcher.py b/reboot/dashboard/api_watcher.py index 7646ce2a..465fe06c 100644 --- a/reboot/dashboard/api_watcher.py +++ b/reboot/dashboard/api_watcher.py @@ -13,12 +13,10 @@ 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 @@ -87,14 +85,17 @@ def retain(self, filenames: set[str]) -> None: if stored not in filenames: del self._errors[stored] - def state_types(self) -> list[StateTypeInfo]: + def state_types(self) -> list[dict]: described = [] for filename in sorted(self._state_types): - for state_type in self._state_types[filename]: - described.append(ParseDict(state_type, StateTypeInfo())) + described.extend(self._state_types[filename]) return described - def error(self) -> str: + def error(self) -> Optional[str]: + """Why the files that failed to read failed, or `None` if none + did.""" + if not self._errors: + return None return '\n'.join( f'{filename}: {self._errors[filename]}' for filename in sorted(self._errors) @@ -130,16 +131,19 @@ 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(), - ) + if current == updated: + return + updated = current + state_types, error = current + + async def update(state: API.State) -> None: + ParseDict(state_types, state.state_types) + if error is None: + state.ClearField('error') + else: + state.error = error + + await API.ref().per_iteration(alias).write(context, update) # Everything, once: the developer may have written the whole API # before the dashboard started. After this only what changes is diff --git a/reboot/dashboard/frontend/BUILD.bazel b/reboot/dashboard/frontend/BUILD.bazel index 6e5b62ed..e8e207e0 100644 --- a/reboot/dashboard/frontend/BUILD.bazel +++ b/reboot/dashboard/frontend/BUILD.bazel @@ -10,18 +10,24 @@ ts_project( name = "dashboard_ts", srcs = [ "src/constants.ts", + "src/description.ts", "src/main.tsx", ], + declaration = True, tsconfig = ":tsconfig", + # `description.ts` is what the contract test parses with. + visibility = ["//tests/reboot/dashboard:__pkg__"], 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/@stoplight/json-schema-tree", "//:node_modules/react", "//:node_modules/react-dom", "//:node_modules/uuid", + "//:node_modules/zod", "//rbt/dashboard/v1:dashboard_js_reboot_react", ], ) diff --git a/reboot/dashboard/frontend/dashboard.css b/reboot/dashboard/frontend/dashboard.css index 99ce79fa..007dc577 100644 --- a/reboot/dashboard/frontend/dashboard.css +++ b/reboot/dashboard/frontend/dashboard.css @@ -23,7 +23,6 @@ --border-soft: 240 5.9% 94%; --surface-sunken: 240 4.8% 97%; --prose: 211 40% 30%; - --returns: 166 47% 33%; --errors: 0 62% 45%; } @@ -401,11 +400,15 @@ header h1 { gap: 14px; } +/* Deliberately not a scroll container. Nothing inside a card paints + out to its edges, so there is no corner for `overflow: hidden` to + clip, and it would clip two things that must escape: a pill's + tooltip, which opens above the card, and the subgrid below, which + stops propagating at a scroll container. */ .method { border: 1px solid hsl(var(--border)); border-radius: var(--radius); background: hsl(var(--card)); - overflow: hidden; } .method-head { @@ -458,14 +461,6 @@ header h1 { 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; } @@ -629,6 +624,14 @@ header h1 { visibility: visible; } +/* Opened downward instead, when the pane has been scrolled to leave + no room above the pill. `Pill` decides which, since only it can + measure. */ +.definition.below { + top: calc(100% + 8px); + bottom: auto; +} + /* 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. */ @@ -683,25 +686,51 @@ header h1 { text-wrap: pretty; } -.method-signature { +.method-types { display: flex; - align-items: baseline; - gap: 12px; - flex-wrap: wrap; + flex-direction: column; + gap: 10px; padding: 10px 18px; background: hsl(var(--surface-sunken)); - border-top: 1px solid hsl(var(--border-soft)); - font-family: ui-monospace, Menlo, monospace; +} + +.type { + display: flex; + flex-direction: column; + gap: 6px; +} + +.type-name { font-size: 11.5px; color: hsl(var(--muted-foreground)); } -.arrow { - color: hsl(240 3.8% 65%); +.type-name code { + font-family: ui-monospace, Menlo, monospace; + color: hsl(var(--foreground)); } -.returns { - color: hsl(var(--returns)); + +/* A type held by a field, drawn under it and stepped in, so depth is + read down the left edge rather than counted. */ +.fields.nested { + margin-left: 14px; + padding-left: 10px; + border-left: 1px solid hsl(240 5.9% 90%); } -.errors { - color: hsl(var(--errors)); + +.field-group { + display: flex; + flex-direction: column; + gap: 6px; +} + +.field-description { + font-size: 11.5px; + color: hsl(var(--muted-foreground)); +} + +/* `Optional[X]` reaches the page as a union with null; the question + mark says the same thing in the space of one character. */ +.optional { + color: hsl(var(--muted-foreground)); } diff --git a/reboot/dashboard/frontend/src/description.ts b/reboot/dashboard/frontend/src/description.ts new file mode 100644 index 00000000..37bbddf9 --- /dev/null +++ b/reboot/dashboard/frontend/src/description.ts @@ -0,0 +1,143 @@ +// What the dashboard reads of the developer's API, and how to walk +// the types in it. +// +// The shape is written down once, here, as a Zod schema: `z.infer` +// gives the page its types, and a test parses the reader's real +// output through it, which is what keeps `api_reader.py` and this +// file in step: the description travels as a `google.protobuf.Value`, +// so nothing generated describes what is inside it. +import { SchemaTree } from "@stoplight/json-schema-tree"; +import { z } from "zod"; + +// JSON Schema is recursive and open-ended, so it is carried through +// rather than described: what the page needs from it is resolved by +// `SchemaTree` below. +const JsonSchema = z.record(z.string(), z.unknown()); + +const Ref = z.object({ $ref: z.string() }); + +export const MethodSchema = z.object({ + name: z.string(), + kind: z.string(), + factory: z.boolean(), + mcp: z.boolean(), + errors: z.array(Ref), + request: Ref.optional(), + response: Ref.optional(), + description: z.string().optional(), +}); + +export const StateTypeSchema = z.object({ + name: z.string(), + file: z.string(), + state: Ref, + methods: z.array(MethodSchema), + $defs: z.record(z.string(), JsonSchema), + description: z.string().optional(), +}); + +export const DescriptionSchema = z.array(StateTypeSchema); + +export type Ref = z.infer; +export type Method = z.infer; +export type StateType = z.infer; +export type Description = z.infer; + +// Takes what `Value.toJson()` gives, which is `undefined` when the +// field is unset. +export const parseDescription = (json: unknown): Description => { + const description = DescriptionSchema.safeParse(json); + // A description this page cannot read is the same to it as no + // description: the last one read stays on the page. + return description.success ? description.data : []; +}; + +// One row of a type, as the page draws it. +export interface Field { + name: string; + type: string; + optional: boolean; + description?: string; + children: Field[]; +} + +const NULL_TYPE = "null"; + +// `SchemaTree` cannot start from a `$ref` at its root, so the +// referred schema is inlined with the pool beside it. +const schemaAt = (stateType: StateType, ref: Ref): object | undefined => { + const name = ref.$ref.replace("#/$defs/", ""); + const schema = stateType.$defs[name]; + return schema === undefined + ? undefined + : { ...(schema as object), $defs: stateType.$defs }; +}; + +const nameOf = (node: any): string => { + const path = node.subpath ?? []; + return path.length > 0 ? String(path[path.length - 1]) : ""; +}; + +const typesOf = (node: any): string[] => node.types ?? []; + +// `Optional[X]` reaches us as `anyOf: [X, null]`, which is a union to +// JSON Schema and an optional field to the person reading the page. +const collapse = (node: any): { node: any; optional: boolean } => { + const children = node.children ?? []; + const branches = children.filter( + (child: any) => !typesOf(child).includes(NULL_TYPE) + ); + const nullable = children.length !== branches.length; + + if (nullable && branches.length === 1 && typesOf(node).length === 0) { + return { node: branches[0], optional: true }; + } + return { node, optional: false }; +}; + +const spell = (node: any): string => { + const types = typesOf(node); + if (types.includes("array")) { + const item = (node.children ?? [])[0]; + return item === undefined ? "array" : `${spell(collapse(item).node)}[]`; + } + if (types.includes("object")) { + return node.title ?? "object"; + } + return types.length > 0 ? types.join(" | ") : "any"; +}; + +const rowsOf = (node: any, depth: number): Field[] => { + if (depth > 8 || !typesOf(node).includes("object")) { + return []; + } + return (node.children ?? []).map((child: any) => { + const { node: value, optional } = collapse(child); + return { + name: nameOf(child), + type: spell(value), + optional, + description: value.annotations?.description, + children: rowsOf( + typesOf(value).includes("array") + ? collapse((value.children ?? [])[0] ?? value).node + : value, + depth + 1 + ), + }; + }); +}; + +// The fields of whatever `ref` points at, nested types followed. +export const fieldsOf = (stateType: StateType, ref: Ref): Field[] => { + const schema = schemaAt(stateType, ref); + if (schema === undefined) { + return []; + } + + const tree = new SchemaTree(schema as any, { mergeAllOf: true }); + tree.populate(); + + const root = (tree.root.children ?? [])[0]; + return root === undefined ? [] : rowsOf(root, 0); +}; diff --git a/reboot/dashboard/frontend/src/main.tsx b/reboot/dashboard/frontend/src/main.tsx index 5c8a883f..dc78ea76 100644 --- a/reboot/dashboard/frontend/src/main.tsx +++ b/reboot/dashboard/frontend/src/main.tsx @@ -1,4 +1,3 @@ -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"; @@ -15,6 +14,8 @@ import { import { createRoot } from "react-dom/client"; import { v4 as uuidv4 } from "uuid"; import { API_ID, PREFERENCES_ID, PRESENCE_ID } from "./constants"; +import type { Field, Method, Ref, StateType } from "./description"; +import { fieldsOf, parseDescription } from "./description"; // One subscriber per tab, for as long as the tab is open. const SUBSCRIBER_ID = uuidv4(); @@ -46,26 +47,66 @@ const DEFINITIONS: Record = { "change them. You can have as many of these as you want.", }; +// The gap between a pill and its definition, matching the offset +// `.definition` is placed at. +const DEFINITION_GAP = 8; + // A pill, with its definition a hover away when it has one. The // small mark is what says there is something to hover. +// +// The definition opens above the pill, where it does not cover the +// row being read, unless the pane has been scrolled to leave no room +// above: the pane clips whatever leaves it, so a pill against its top +// edge would show nothing at all. Then it opens downward instead. const Pill: FC<{ className: string; label: string; meaning?: string }> = ({ className, label, meaning, -}) => - meaning === undefined ? ( - {label} - ) : ( - +}) => { + const pill = useRef(null); + const [below, setBelow] = useState(false); + + // Measured on the way in rather than on every scroll, since where + // it opens only matters at the moment it opens. The definition is + // hidden by `visibility`, so it has a height to read while closed; + // and the room is measured from the pill rather than from the + // definition, whose own position is what this decides. + const place = useCallback(() => { + const pane = pill.current?.closest(".pane"); + const definition = pill.current?.querySelector(".definition"); + if (pane == null || definition == null) { + return; + } + const room = + pill.current!.getBoundingClientRect().top - + pane.getBoundingClientRect().top; + setBelow(room < definition.getBoundingClientRect().height + DEFINITION_GAP); + }, []); + + if (meaning === undefined) { + return {label}; + } + + return ( + {label} - + {meaning} ); +}; const Kind: FC<{ kind: string }> = ({ kind }) => ( const isStandardLibrary = (namespace: string): boolean => namespace.startsWith("rbt."); -const Namespace: FC<{ namespace: string; types: StateTypeInfo[] }> = ({ +const Namespace: FC<{ namespace: string; types: StateType[] }> = ({ namespace, types, }) => { @@ -146,17 +187,52 @@ const Namespace: FC<{ namespace: string; types: StateTypeInfo[] }> = ({ ); }; -const Method: FC<{ method: MethodInfo }> = ({ method }) => { - const args = method.arguments - .map((argument) => `${argument.name}: ${argument.type}`) - .join(", "); +// A type's fields, and the fields of any type they hold. Nesting is +// what this page is for: a field whose type is another type is not a +// dead end, it opens. +const Fields: FC<{ fields: Field[]; depth?: number }> = ({ + fields, + depth = 0, +}) => ( +
+ {fields.map((field) => ( +
+
+ {field.name} + + {field.type} + {field.optional && ?} + + {field.description !== undefined && ( + {field.description} + )} +
+ {field.children.length > 0 && ( + + )} +
+ ))} +
+); - // 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(", "); +// A request, response or error, named and opened. +const TypeOf: FC<{ stateType: StateType; label: string; type: Ref }> = ({ + stateType, + label, + type, +}) => ( +
+
+ {label} {type.$ref.replace("#/$defs/", "")} +
+ +
+); +const Method: FC<{ stateType: StateType; method: Method }> = ({ + stateType, + method, +}) => { return (
@@ -200,16 +276,33 @@ const Method: FC<{ method: MethodInfo }> = ({ method }) => { text={method.description} /> )} -
- - ({args}) {" "} - - {method.returns.length > 0 ? `{${returns}}` : "None"} - - - {method.errors.length > 0 && ( - raises {method.errors.join(", ")} +
+ {method.request !== undefined ? ( + + ) : ( +
takes nothing
+ )} + {method.response !== undefined ? ( + + ) : ( +
returns nothing
)} + {method.errors.map((error) => ( + + ))}
@@ -280,11 +373,12 @@ const useSlidingPills = (expanded: boolean) => { }; const StateType: FC<{ - stateType: StateTypeInfo; + stateType: StateType; expanded: boolean; onToggle: () => void; }> = ({ stateType, expanded, onToggle }) => { const section = useSlidingPills(expanded); + const fields = fieldsOf(stateType, stateType.state); return ( // Every method's detail opens and closes off this one class, so a @@ -305,7 +399,7 @@ const StateType: FC<{

{typeNameOf(stateType.name)}

- {countOf(stateType.fields.length, "field")} ·{" "} + {countOf(fields.length, "field")} ·{" "} {countOf(stateType.methods.length, "method")}
@@ -330,25 +424,18 @@ const StateType: FC<{ )}
state
- {stateType.fields.length === 0 ? ( + {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) => ( - + ))}
@@ -383,18 +470,24 @@ const Overview: FC<{ // 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; + // The description travels as a `google.protobuf.Value`: the types + // in it are Pydantic's own JSON Schema, which proto has no business + // restating. + const read = useMemo( + () => parseDescription(response?.stateTypes?.toJson()), + [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([]); + const seen = useRef([]); - if (read !== undefined && read.length > 0) { + if (read.length > 0) { seen.current = read; } - const stateTypes: StateTypeInfo[] = read?.length ? read : seen.current; + const stateTypes: StateType[] = 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 @@ -402,7 +495,7 @@ const Overview: FC<{ const error = response?.error ?? ""; const namespaces = useMemo(() => { - const byNamespace = new Map(); + const byNamespace = new Map(); for (const stateType of stateTypes) { const namespace = namespaceOf(stateType.name); const types = byNamespace.get(namespace); diff --git a/reboot/dashboard/frontend/tsconfig.json b/reboot/dashboard/frontend/tsconfig.json index 3446f50f..af959a42 100644 --- a/reboot/dashboard/frontend/tsconfig.json +++ b/reboot/dashboard/frontend/tsconfig.json @@ -5,6 +5,8 @@ "jsx": "react-jsx", "moduleResolution": "bundler", "skipLibCheck": true, + // So the contract test can import `description.ts` by name. + "declaration": true, "verbatimModuleSyntax": true, "baseUrl": ".", "paths": { diff --git a/reboot/dashboard/servicers.py b/reboot/dashboard/servicers.py index dec9cd41..40ae01f7 100644 --- a/reboot/dashboard/servicers.py +++ b/reboot/dashboard/servicers.py @@ -33,8 +33,11 @@ async def Get( request: APIGetRequest, ) -> APIGetResponse: return APIGetResponse( - state_types=self.state.state_types, - error=self.state.error, + state_types=( + self.state.state_types + if self.state.HasField('state_types') else None + ), + error=self.state.error if self.state.HasField('error') else None, ) @classmethod @@ -61,9 +64,14 @@ async def Update( context: WriterContext, request: APIUpdateRequest, ) -> APIUpdateResponse: - del self.state.state_types[:] - self.state.state_types.extend(request.state_types) - self.state.error = request.error + if request.HasField('state_types'): + self.state.state_types.CopyFrom(request.state_types) + else: + self.state.ClearField('state_types') + if request.HasField('error'): + self.state.error = request.error + else: + self.state.ClearField('error') return APIUpdateResponse() diff --git a/reboot/plugin/skills/chat-app/references/api-method-types.md b/reboot/plugin/skills/chat-app/references/api-method-types.md index f48c019a..ce73b037 100644 --- a/reboot/plugin/skills/chat-app/references/api-method-types.md +++ b/reboot/plugin/skills/chat-app/references/api-method-types.md @@ -35,6 +35,8 @@ Every method must explicitly declare its MCP exposure: - **`Tool(name="...", title="...")`** — override the default tool name or add a human-readable title. +A tool's description comes from the method's `description`. + `Workflow(...)` requires `mcp=` like every other method factory; usually `None` since workflows are rarely AI-callable tools directly. Omitting `mcp=` raises at codegen with @@ -162,6 +164,10 @@ api = API( ), Counter=Type( state=CounterState, + # What the state type is for, shown by the dev dashboard + # beside its name and file. + description="One counter the user created, and the " + "consistency boundary its value changes on.", methods=Methods( # `show_clicker` lives on `Counter` (not `User`) # because it shows ONE specific Counter. The AI calls @@ -175,10 +181,14 @@ api = API( title="Counter Clicker", description="Interactive clicker UI for the counter.", ), + # Not an MCP tool, and still worth describing: the dev + # dashboard shows `description=` for every method. create=Writer( request=None, response=None, factory=True, + description="Create the counter at zero. Called by " + "`User.create_counter`, not by the AI.", mcp=None, ), get=Reader( diff --git a/reboot/plugin/skills/python/references/api-methods.md b/reboot/plugin/skills/python/references/api-methods.md index bbc6335e..7c19e626 100644 --- a/reboot/plugin/skills/python/references/api-methods.md +++ b/reboot/plugin/skills/python/references/api-methods.md @@ -177,6 +177,35 @@ withdraw=Writer( See `api-errors.md` for raising and catching them. +## `description=` Says What the Method Does + +All four method kinds take an optional `description=`: + +```python +balance=Reader( + request=None, + response=BalanceResponse, + description="The funds currently available to withdraw.", + mcp=None, +), +withdraw=Writer( + request=WithdrawRequest, + response=None, + errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", + mcp=Tool(), +), +``` + +The dev dashboard shows it, and `mcp=Tool()` methods use it as the +tool's description. Write what a caller cannot derive from the +signature: the precondition, the side effect, the unit, which error it +raises and when. + +A state type takes one too, via `Type(description=...)`; see +`api-pydantic.md`. + ## Every Factory Takes `mcp=` All four factories require an explicit `mcp=` keyword. Use diff --git a/reboot/plugin/skills/python/references/api-pydantic.md b/reboot/plugin/skills/python/references/api-pydantic.md index a0a9ff17..ced25254 100644 --- a/reboot/plugin/skills/python/references/api-pydantic.md +++ b/reboot/plugin/skills/python/references/api-pydantic.md @@ -118,7 +118,8 @@ api = API( Attach typed errors via `errors=[ErrorModel, ...]`. Mark a method as a constructor with `factory=True` (only valid on `Writer` and -`Transaction`, see below). +`Transaction`, see below). Say what the method does with +`description="..."`; see `api-methods.md`. ### `factory=True` Only Works on `Writer` and `Transaction` @@ -316,6 +317,29 @@ with `.Servicer`, `.ref(id)`, and request/response messages nested as attributes (`Account.BalanceResponse`, `Account.DepositRequest`, etc.). +### `Type(description=...)` Says What the State Type Is For + +`Type` takes an optional `description=`, shown by the dev dashboard +beside the state type's name and file: + +```python +api = API( + Account=Type( + state=AccountState, + methods=AccountMethods, + description="One customer's money, and the consistency " + "boundary every balance change is serialized on.", + ), +) +``` + +A state type is the sum of its state and its methods, and its name +alone rarely says what it is _for_. Write about the part a reader +cannot derive from the fields and methods listed beside it: what it +is the consistency boundary for, what one instance corresponds to, +how its ID is chosen. Restating them adds nothing. Per-method +descriptions are separate; see `api-methods.md`. + ### Generated Request/Response Names Come From the **Method Name** The codegen names the nested attributes after the **method name** in diff --git a/reboot/plugin/skills/upgrade/migrations/next/method-description-out-of-mcp-options.md b/reboot/plugin/skills/upgrade/migrations/next/method-description-out-of-mcp-options.md new file mode 100644 index 00000000..69ac269d --- /dev/null +++ b/reboot/plugin/skills/upgrade/migrations/next/method-description-out-of-mcp-options.md @@ -0,0 +1,78 @@ +## `description` moved out of the `mcp` options block + +A method's description now belongs on the method options themselves +rather than inside the nested `mcp` options, so that a `reader`, +`writer`, `transaction` or `workflow` which is **not** exposed to MCP +can have one too. `McpMethodOptions.description` is marked +`deprecated`; it is still read when it is the only description +present, so nothing breaks today, but it will stop being read, and a +description left there is invisible to everything that is not an MCP +tool or resource, the dev dashboard included. + +**This applies only to hand-written `.proto` API files.** An +application whose API is defined in Pydantic (`reboot.api`) or in Zod +already writes `description=` on the method itself, and `rbt generate` +puts it in the new place. Those applications have nothing to change +here. + +In every `.proto` under the application's API directories (the +directories `rbt generate` is pointed at in `.rbtrc`), find each +`description:` that sits inside an `mcp: {` / `mcp = {` block, and +move it out into the enclosing method options. Everything else +(`tool:`, `resource:`, `name:`, `title:`) stays where it is, and the +description text itself is unchanged. + +Two option spellings are in use, and each moves differently. + +**Full form.** The `description:` line moves up one level, out of +`mcp: { ... }` and into `option (rbt.v1alpha1.method) = { ... }`: + +```proto +// Before. +option (rbt.v1alpha1.method) = { + writer: {}, + mcp: { + tool: true, + description: "Reply to a message", + }, +}; + +// After. +option (rbt.v1alpha1.method) = { + writer: {}, + description: "Reply to a message", + mcp: { + tool: true, + }, +}; +``` + +**Shorthand form.** `option (rbt.v1alpha1.method).mcp = { ... }` +cannot carry the description, because the description is no longer a +field of `mcp`. Drop the line from the `mcp` block and add a sibling +`option (rbt.v1alpha1.method).description = "...";` statement: + +```proto +// Before. +option (rbt.v1alpha1.method).writer = {}; +option (rbt.v1alpha1.method).mcp = { + tool: true, + description: "Reply to a message", +}; + +// After. +option (rbt.v1alpha1.method).writer = {}; +option (rbt.v1alpha1.method).description = "Reply to a message"; +option (rbt.v1alpha1.method).mcp = { + tool: true, +}; +``` + +Do not delete an `mcp` block that is left holding only `tool: true` +(or `resource: true`); that field is what exposes the method to MCP. +Delete the block only if removing `description:` empties it entirely, +which means the method was never exposed to MCP in the first place. + +The generated MCP tool and resource descriptions are unchanged by this +move: they now read the method's description, falling back to the same +default text as before when there is none. diff --git a/tests/reboot/cli/dashboard_tests.py b/tests/reboot/cli/dashboard_tests.py index f5bea4ff..24dbf061 100644 --- a/tests/reboot/cli/dashboard_tests.py +++ b/tests/reboot/cli/dashboard_tests.py @@ -48,6 +48,8 @@ async def test_env_is_isolated_from_any_application(self) -> None: 'RBT_STATE_DIRECTORY': '/somewhere/app', 'RBT_NODEJS': 'true', 'REBOOT_LOCAL_ENVOY_PORT': '9991', + 'RBT_DEV': 'true', + 'RBT_EFFECT_VALIDATION': 'ENABLED', }, ): env = dashboard._dashboard_env( @@ -70,6 +72,14 @@ async def test_env_is_isolated_from_any_application(self) -> None: self.assertEqual(env['RBT_SERVERS'], '1') self.assertEqual(env['REBOOT_LOCAL_ENVOY'], 'true') + # `rbt serve` defaults, not `rbt dev` ones: `RBT_SERVE` + # alone is not enough to produce a `rbt serve` + # environment, and `RBT_DEV` has to be gone rather than + # merely unset, since it is read first. + self.assertEqual(env['RBT_SERVE'], 'true') + self.assertNotIn('RBT_DEV', env) + self.assertEqual(env['RBT_EFFECT_VALIDATION'], 'DISABLED') + # A sibling of `.rbt/dev/`, so that it can never collide # with an application's state at `.rbt/dev//`. self.assertEqual( diff --git a/tests/reboot/dashboard/BUILD.bazel b/tests/reboot/dashboard/BUILD.bazel index c9956e23..018c1abd 100644 --- a/tests/reboot/dashboard/BUILD.bazel +++ b/tests/reboot/dashboard/BUILD.bazel @@ -1,3 +1,5 @@ +load("@aspect_rules_ts//ts:defs.bzl", "ts_project") +load("@fremtind_rules_vitest//vitest:defs.bzl", "vitest_test") load("@rules_python//python:defs.bzl", "py_test") load("//tests/reboot/react:py_web_test_suite_env.bzl", "py_web_test_suite_env") @@ -81,3 +83,56 @@ py_test( "//reboot/dashboard:main_py", ], ) + +# `api_reader.py` writes the description and `description.ts` reads +# it. It travels as JSON, so no generated type holds the two +# languages to the same shape; this does. The real reader runs over +# the API files beside this test, and the page's own Zod schema +# parses what it wrote. +# Emitted as TypeScript rather than JSON so that it is simply a +# source of the test below, with no file for it to find at runtime. +genrule( + name = "described", + srcs = glob(["api/**"]), + outs = ["described.ts"], + cmd = "echo 'export default ' > $@ && " + + "$(location //reboot/dashboard:api_reader) " + + "$$(dirname $(location api/shop/v1/shop.py))/../.. " + + "shop/v1/shop.py >> $@", + tools = ["//reboot/dashboard:api_reader"], +) + +ts_project( + name = "description_test_ts", + srcs = [ + "description.test.ts", + ":described.ts", + ], + declaration = True, + tsconfig = { + "compilerOptions": { + "declaration": True, + "module": "esnext", + "moduleResolution": "bundler", + "skipLibCheck": True, + "target": "es2020", + }, + }, + deps = [ + "//:node_modules/@stoplight/json-schema-tree", + "//:node_modules/zod", + "//reboot/dashboard/frontend:dashboard_ts", + "//reboot/std/vitest:vitest_ts", + ], +) + +vitest_test( + name = "description_test", + config = "vitest.config.mjs", + data = [ + "package.json", + ":description_test_ts", + ], + node_modules = "//:node_modules", + visibility = ["//visibility:public"], +) diff --git a/tests/reboot/dashboard/api/shop/v1/shop.py b/tests/reboot/dashboard/api/shop/v1/shop.py index efa1ca60..5cfeb02e 100644 --- a/tests/reboot/dashboard/api/shop/v1/shop.py +++ b/tests/reboot/dashboard/api/shop/v1/shop.py @@ -8,6 +8,7 @@ Transaction, Type, ) +from typing import Optional class ShopState(Model): @@ -20,8 +21,21 @@ class StockRequest(Model): quantity: int = Field(tag=2) +class Price(Model): + """What one item costs.""" + currency: str = Field(tag=1) + cents: int = Field(tag=2) + + +class Item(Model): + """One thing the shop sells.""" + name: str = Field(tag=1) + price: Optional[Price] = Field(tag=2, default=None) + + class StockResponse(Model): remaining: int = Field(tag=1) + items: list[Item] = Field(tag=2, default_factory=list) class OutOfStockError(Model): diff --git a/tests/reboot/dashboard/api_reader_tests.py b/tests/reboot/dashboard/api_reader_tests.py index 9cf31d72..d9a4c1ec 100644 --- a/tests/reboot/dashboard/api_reader_tests.py +++ b/tests/reboot/dashboard/api_reader_tests.py @@ -17,6 +17,10 @@ def _by_name(state_types: list[dict]) -> dict[str, dict]: return {state_type['name']: state_type for state_type in state_types} +def _defs(state_type: dict, name: str) -> dict: + return state_type['$defs'][name] + + def _method(state_type: dict, name: str) -> dict: for method in state_type['methods']: if method['name'] == name: @@ -46,8 +50,10 @@ async def test_describes_a_state_type_and_its_methods(self) -> None: 'A shop, and the stock it has to sell.', ) + # The state is a `$ref` into this state type's own `$defs`. + self.assertEqual(shop['state'], {'$ref': '#/$defs/ShopState'}) self.assertEqual( - [field['name'] for field in shop['fields']], + list(_defs(shop, 'ShopState')['properties']), ['name', 'open'], ) @@ -55,8 +61,9 @@ async def test_describes_a_state_type_and_its_methods(self) -> None: # their author wrote. stock = _method(shop, 'stock') self.assertEqual(stock['kind'], 'transaction') + self.assertEqual(stock['request'], {'$ref': '#/$defs/StockRequest'}) self.assertEqual( - [argument['name'] for argument in stock['arguments']], + list(_defs(shop, 'StockRequest')['properties']), ['item', 'quantity'], ) @@ -64,31 +71,83 @@ async def test_describes_a_state_type_and_its_methods(self) -> None: # 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) + self.assertFalse(stock['mcp']) remaining = _method(shop, 'remaining') self.assertEqual(remaining['kind'], 'reader') self.assertEqual( - remaining['returns'], - [{ - 'name': 'remaining', - 'type': 'int', - }], + remaining['response'], + {'$ref': '#/$defs/StockResponse'}, ) 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. + # An error is a `$ref` like anything else, so what it holds + # can be read rather than only its name. + self.assertEqual( + remaining['errors'], + [{ + '$ref': '#/$defs/OutOfStockError' + }], + ) + self.assertEqual( + list(_defs(shop, 'OutOfStockError')['properties']), + ['item'], + ) + + # A factory constructs the state, and takes and returns + # nothing. create = _method(shop, 'create') self.assertTrue(create['factory']) - self.assertNotIn('returns', create) + self.assertNotIn('request', create) + self.assertNotIn('response', create) + + async def test_a_nested_type_is_followed_rather_than_named(self) -> None: + # The whole point: a type that holds another type is not a + # dead end. `StockResponse.items` is a list of `Item`, whose + # `price` is an `Optional[Price]`, and every one of those is + # in `$defs` to be read. + state_types, error = await read(API_DIRECTORY, 'shop/v1/shop.py') + + self.assertIsNone(error) + shop = _by_name(state_types)['shop.v1.Shop'] + + self.assertEqual( + _defs(shop, 'StockResponse')['properties']['items'], + { + 'items': { + '$ref': '#/$defs/Item' + }, + 'tag': 2, + 'title': 'Items', + 'type': 'array', + }, + ) + + # `Optional[X]` is spelled as a union with null. + self.assertEqual( + _defs(shop, 'Item')['properties']['price']['anyOf'], + [{ + '$ref': '#/$defs/Price' + }, { + 'type': 'null' + }], + ) + + self.assertEqual( + list(_defs(shop, 'Price')['properties']), + ['currency', 'cents'], + ) + + # A model's docstring describes it, so prose the author + # already wrote reaches the page. + self.assertEqual( + _defs(shop, 'Item')['description'], + 'One thing the shop sells.', + ) async def test_a_file_with_no_api_describes_nothing(self) -> None: # A directory holds shared code as well as APIs, and reading a diff --git a/tests/reboot/dashboard/api_watcher_tests.py b/tests/reboot/dashboard/api_watcher_tests.py index e5fa6b84..09b92221 100644 --- a/tests/reboot/dashboard/api_watcher_tests.py +++ b/tests/reboot/dashboard/api_watcher_tests.py @@ -8,6 +8,7 @@ import os import tempfile import unittest +from google.protobuf.json_format import MessageToDict from pathlib import Path from rbt.dashboard.v1.dashboard_rbt import API from reboot.aio.tests import Reboot @@ -45,6 +46,13 @@ class LookResponse(Model): ''' +def _read(response) -> list[dict]: + """The state types the description carries, as JSON.""" + if not response.HasField('state_types'): + return [] + return MessageToDict(response.state_types) + + class APIWatcherTest(unittest.IsolatedAsyncioTestCase): watcher: Optional[asyncio.Task] = None @@ -91,18 +99,18 @@ async def test_types_appear_as_files_are_written(self) -> None: # application came up. self._write(self.directory, 'shop', 'Shop') - response = await self._wait_for(lambda api: len(api.state_types) == 1) + response = await self._wait_for(lambda api: len(_read(api)) == 1) self.assertEqual( - [state.name for state in response.state_types], + [state['name'] for state in _read(response)], ['shop.v1.Shop'], ) - self.assertEqual(response.error, '') + self.assertFalse(response.HasField('error')) self._write(self.directory, 'depot', 'Depot') - response = await self._wait_for(lambda api: len(api.state_types) == 2) + response = await self._wait_for(lambda api: len(_read(api)) == 2) self.assertEqual( - sorted(state.name for state in response.state_types), + sorted(state['name'] for state in _read(response)), ['shop.v1.Depot', 'shop.v1.Shop'], ) diff --git a/tests/reboot/dashboard/dashboard_tests.py b/tests/reboot/dashboard/dashboard_tests.py index 67b1f6c7..3e45bb4e 100644 --- a/tests/reboot/dashboard/dashboard_tests.py +++ b/tests/reboot/dashboard/dashboard_tests.py @@ -8,7 +8,8 @@ import asyncio import socket import unittest -from rbt.dashboard.v1.dashboard_pb2 import FieldInfo, MethodInfo, StateTypeInfo +from google.protobuf.json_format import ParseDict +from google.protobuf.struct_pb2 import Value from rbt.dashboard.v1.dashboard_rbt import API, Preferences from reboot.aio.tests import Reboot from reboot.dashboard.constants import ( @@ -44,6 +45,63 @@ def _driver(): ) +# One state type, spelled the way `api_reader` spells it: types by +# `$ref` into the state type's own `$defs`. +_SHOP = { + 'name': 'shop.v1.Shop', + 'file': 'api/shop/v1/shop.py', + 'state': { + '$ref': '#/$defs/ShopState' + }, + 'methods': + [ + { + 'name': 'look', + 'kind': 'reader', + 'factory': False, + 'mcp': False, + 'errors': [], + 'request': { + '$ref': '#/$defs/LookRequest' + }, + 'response': { + '$ref': '#/$defs/LookResponse' + }, + }, + ], + '$defs': + { + 'ShopState': + { + 'type': 'object', + 'properties': { + 'name': { + 'type': 'string' + } + }, + }, + 'LookRequest': + { + 'type': 'object', + 'properties': { + 'item': { + 'type': 'string' + } + }, + }, + 'LookResponse': + { + 'type': 'object', + 'properties': { + 'found': { + 'type': 'boolean' + } + }, + }, + }, +} + + class DashboardTest(unittest.IsolatedAsyncioTestCase): async def asyncSetUp(self) -> None: @@ -122,22 +180,7 @@ async def _record_state_types(self) -> None: 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='', + state_types=ParseDict([_SHOP], Value()), ) def _run(self, body): @@ -200,13 +243,7 @@ def body(driver): 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')], - ), - ], + state_types=ParseDict([_SHOP], Value()), error='shop.py: SyntaxError: invalid syntax', ) diff --git a/tests/reboot/dashboard/description.test.ts b/tests/reboot/dashboard/description.test.ts new file mode 100644 index 00000000..a59e5d98 --- /dev/null +++ b/tests/reboot/dashboard/description.test.ts @@ -0,0 +1,55 @@ +// The reader and the page agree on a shape. +// +// `api_reader.py` writes the description and `description.ts` reads +// it, and since it travels as JSON there is no generated type holding +// the two together. This is what does instead: the build runs the +// real reader over `api/`, and parsing what it wrote fails if either +// side has drifted from the other. +import { describe, expect, it } from "vitest"; +import { + DescriptionSchema, + fieldsOf, +} from "../../../reboot/dashboard/frontend/src/description"; +import described from "./described"; + +describe("the description the reader writes", () => { + it("is what this page expects", () => { + expect(() => DescriptionSchema.parse(described)).not.toThrow(); + }); + + it("carries nested types rather than naming them", () => { + const [shop] = DescriptionSchema.parse(described); + + const remaining = shop.methods.find( + (method) => method.name === "remaining" + ); + expect(remaining?.response).toBeDefined(); + + const fields = fieldsOf(shop, remaining!.response!); + const items = fields.find((field) => field.name === "items"); + + // `items` is a list of `Item`, and `Item` opens: its `price` is + // an `Optional[Price]`, and `Price` opens in turn. This is the + // whole point of the change. + expect(items?.type).toBe("Item[]"); + + const price = items?.children.find((field) => field.name === "price"); + expect(price?.optional).toBe(true); + expect(price?.children.map((field) => field.name)).toEqual([ + "currency", + "cents", + ]); + }); + + it("makes an error's fields readable, not just its name", () => { + const [shop] = DescriptionSchema.parse(described); + + const remaining = shop.methods.find( + (method) => method.name === "remaining" + ); + const [error] = remaining!.errors; + + expect(error.$ref).toBe("#/$defs/OutOfStockError"); + expect(fieldsOf(shop, error).map((field) => field.name)).toEqual(["item"]); + }); +}); diff --git a/tests/reboot/dashboard/package.json b/tests/reboot/dashboard/package.json new file mode 100644 index 00000000..3dbc1ca5 --- /dev/null +++ b/tests/reboot/dashboard/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/tests/reboot/dashboard/vitest.config.mjs b/tests/reboot/dashboard/vitest.config.mjs new file mode 100644 index 00000000..144cceef --- /dev/null +++ b/tests/reboot/dashboard/vitest.config.mjs @@ -0,0 +1,6 @@ +export default { + test: { + globals: true, + environment: "node", + }, +}; From 117ac7c36bf000a1206099bedc1c0cef71791666 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 18 Aug 2026 01:57:58 +0000 Subject: [PATCH 7/9] Add 'description' to docs and examples --- documentation/docs/ai_chat_apps/get_started.mdx | 2 ++ documentation/docs/ai_chat_apps/what_is.mdx | 2 ++ documentation/docs/full_stack_apps/python.mdx | 2 ++ documentation/docs/learn_more/call/from_react.mdx | 4 +++- documentation/docs/learn_more/call/overview.mdx | 4 +++- documentation/docs/learn_more/define/pydantic.mdx | 6 ++++++ documentation/docs/learn_more/errors.mdx | 4 +++- reboot/examples/agent-wiki/api/agent_wiki/v1/wiki.py | 6 ++++++ .../api/ai_chat_counter/v1/counter.py | 3 +++ .../ai-chat-counter/api/ai_chat_counter/v1/counter.py | 2 ++ reboot/examples/bank-pydantic/api/bank/v1/account.py | 5 +++++ reboot/examples/bank-pydantic/api/bank/v1/bank.py | 1 + reboot/examples/bank-pydantic/api/bank/v1/customer.py | 6 ++++++ reboot/examples/chick-potle/api/ai_chat_food/v1/food.py | 2 ++ .../reboot-swag-store/api/reboot_swag_store/v1/store.py | 6 ++++++ tests/reboot/documentation/ai_chat_counter.py | 3 +++ tests/reboot/documentation/chat_room_pydantic.py | 2 ++ tests/reboot/documentation/errors_pydantic.py | 2 ++ 18 files changed, 59 insertions(+), 3 deletions(-) diff --git a/documentation/docs/ai_chat_apps/get_started.mdx b/documentation/docs/ai_chat_apps/get_started.mdx index 16e9b581..ab80287a 100644 --- a/documentation/docs/ai_chat_apps/get_started.mdx +++ b/documentation/docs/ai_chat_apps/get_started.mdx @@ -184,6 +184,8 @@ api = API( request=InitializeCounterRequest, response=None, factory=True, + description="Create the counter at its initial value. " + "Called by `User.create_counter`, not by the AI.", mcp=None, ), get=Reader( diff --git a/documentation/docs/ai_chat_apps/what_is.mdx b/documentation/docs/ai_chat_apps/what_is.mdx index 8938e52b..57e93c73 100644 --- a/documentation/docs/ai_chat_apps/what_is.mdx +++ b/documentation/docs/ai_chat_apps/what_is.mdx @@ -148,6 +148,8 @@ api = API( request=InitializeCounterRequest, response=None, factory=True, + description="Create the counter at its initial value. " + "Called by `User.create_counter`, not by the AI.", mcp=None, ), get=Reader( diff --git a/documentation/docs/full_stack_apps/python.mdx b/documentation/docs/full_stack_apps/python.mdx index 6da454c3..3479c168 100644 --- a/documentation/docs/full_stack_apps/python.mdx +++ b/documentation/docs/full_stack_apps/python.mdx @@ -144,10 +144,12 @@ ChatRoomMethods = Methods( messages=Reader( request=None, response=MessagesResponse, + description="Every message posted to the room so far.", ), send=Writer( request=SendRequest, response=None, + description="Post one message to the room.", ), ) diff --git a/documentation/docs/learn_more/call/from_react.mdx b/documentation/docs/learn_more/call/from_react.mdx index e8c4c4cb..1120593e 100644 --- a/documentation/docs/learn_more/call/from_react.mdx +++ b/documentation/docs/learn_more/call/from_react.mdx @@ -53,7 +53,7 @@ export const api = { +(CODE:src=../../../../tests/reboot/documentation/chat_room_pydantic.py&lines=1-39) --> ```py @@ -78,12 +78,14 @@ ChatRoomMethods = Methods( messages=Reader( request=None, response=MessagesResponse, + description="Every message posted to the room so far.", mcp=None, ), # Adds a new message to the list of recorded messages. send=Writer( request=SendRequest, response=None, + description="Post one message to the room.", mcp=None, ), ) diff --git a/documentation/docs/learn_more/call/overview.mdx b/documentation/docs/learn_more/call/overview.mdx index 008c048e..fddeb24b 100644 --- a/documentation/docs/learn_more/call/overview.mdx +++ b/documentation/docs/learn_more/call/overview.mdx @@ -149,7 +149,7 @@ the `Open` method as an explicit constructor for `Account`: - + ```python @@ -159,6 +159,8 @@ AccountMethods = Methods( request=None, response=None, factory=True, + description="Bring the account into existence with a zero " + "balance.", mcp=None, ), ``` diff --git a/documentation/docs/learn_more/define/pydantic.mdx b/documentation/docs/learn_more/define/pydantic.mdx index e80bed2b..d3418fc8 100644 --- a/documentation/docs/learn_more/define/pydantic.mdx +++ b/documentation/docs/learn_more/define/pydantic.mdx @@ -121,6 +121,8 @@ api = API( request=InitializeCounterRequest, response=None, factory=True, + description="Create the counter at its initial value. " + "Called by `User.create_counter`, not by the AI.", mcp=None, ), get=Reader( @@ -235,6 +237,8 @@ CounterMethods = Methods( request=None, response=None, factory=True, + description="Create the counter at zero. Called by " + "`User.create_counter`, not by the AI.", mcp=None, ), ) @@ -324,10 +328,12 @@ ChatRoomMethods = Methods( messages=Reader( request=None, response=MessagesResponse, + description="Every message posted to the room so far.", ), send=Writer( request=SendRequest, response=None, + description="Post one message to the room.", ), ) diff --git a/documentation/docs/learn_more/errors.mdx b/documentation/docs/learn_more/errors.mdx index 7217d39c..be449f0c 100644 --- a/documentation/docs/learn_more/errors.mdx +++ b/documentation/docs/learn_more/errors.mdx @@ -43,7 +43,7 @@ method's `errors`: +(CODE:src=../../../tests/reboot/documentation/errors_pydantic.py&lines=1-24) --> ```py @@ -65,6 +65,8 @@ AccountMethods = Methods( request=WithdrawRequest, response=None, errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", mcp=None, ), # ... diff --git a/reboot/examples/agent-wiki/api/agent_wiki/v1/wiki.py b/reboot/examples/agent-wiki/api/agent_wiki/v1/wiki.py index 1ab7bd99..3754fe6e 100644 --- a/reboot/examples/agent-wiki/api/agent_wiki/v1/wiki.py +++ b/reboot/examples/agent-wiki/api/agent_wiki/v1/wiki.py @@ -205,6 +205,8 @@ class TranscriptUpdateRequest(Model): request=WikiCreateRequest, response=None, factory=True, + description="Bring the Wiki into existence " + "with an empty body.", mcp=None, ), get=Reader( @@ -294,6 +296,8 @@ class TranscriptUpdateRequest(Model): request=PageCreateRequest, response=None, factory=True, + description="Bring the Page into existence " + "with its title and body.", mcp=None, ), get=Reader( @@ -343,6 +347,8 @@ class TranscriptUpdateRequest(Model): request=TranscriptCreateRequest, response=None, factory=True, + description="Record one raw conversation " + "transcript.", mcp=None, ), get=Reader( diff --git a/reboot/examples/ai-chat-counter-dashboard/api/ai_chat_counter/v1/counter.py b/reboot/examples/ai-chat-counter-dashboard/api/ai_chat_counter/v1/counter.py index 8c68bc67..20d2187f 100644 --- a/reboot/examples/ai-chat-counter-dashboard/api/ai_chat_counter/v1/counter.py +++ b/reboot/examples/ai-chat-counter-dashboard/api/ai_chat_counter/v1/counter.py @@ -104,6 +104,9 @@ class DashboardConfig(Model): request=CreateCounterRequest, response=None, factory=True, + description="Create the counter at its " + "initial value. Called by " + "`User.create_counter`, not by the AI.", mcp=None, ), get=Reader( diff --git a/reboot/examples/ai-chat-counter/api/ai_chat_counter/v1/counter.py b/reboot/examples/ai-chat-counter/api/ai_chat_counter/v1/counter.py index 0add64dc..9de359fc 100644 --- a/reboot/examples/ai-chat-counter/api/ai_chat_counter/v1/counter.py +++ b/reboot/examples/ai-chat-counter/api/ai_chat_counter/v1/counter.py @@ -100,6 +100,8 @@ class IncrementRequest(Model): request=InitializeCounterRequest, response=None, factory=True, + description="Create the counter at its initial value. " + "Called by `User.create_counter`, not by the AI.", mcp=None, ), get=Reader( diff --git a/reboot/examples/bank-pydantic/api/bank/v1/account.py b/reboot/examples/bank-pydantic/api/bank/v1/account.py index 12530dd3..7be2fbdf 100644 --- a/reboot/examples/bank-pydantic/api/bank/v1/account.py +++ b/reboot/examples/bank-pydantic/api/bank/v1/account.py @@ -27,6 +27,8 @@ class OverdraftError(Model): request=None, response=None, factory=True, + description="Bring the account into existence with a zero " + "balance.", mcp=None, ), balance=Reader( @@ -56,6 +58,7 @@ class OverdraftError(Model): interest=Writer( request=None, response=None, + description="Credit one period's interest at the current rate.", mcp=None, ), ) @@ -64,5 +67,7 @@ class OverdraftError(Model): Account=Type( state=AccountState, methods=AccountMethods, + description="One account's money, and the consistency boundary " + "for every change to it.", ), ) diff --git a/reboot/examples/bank-pydantic/api/bank/v1/bank.py b/reboot/examples/bank-pydantic/api/bank/v1/bank.py index 74d98ee6..eaebb5b5 100644 --- a/reboot/examples/bank-pydantic/api/bank/v1/bank.py +++ b/reboot/examples/bank-pydantic/api/bank/v1/bank.py @@ -53,6 +53,7 @@ class AccountBalancesResponse(Model): request=None, response=None, factory=True, + description="Bring the bank into existence with no customers.", mcp=None, ), sign_up=Transaction( diff --git a/reboot/examples/bank-pydantic/api/bank/v1/customer.py b/reboot/examples/bank-pydantic/api/bank/v1/customer.py index c59a7f93..b11e27da 100644 --- a/reboot/examples/bank-pydantic/api/bank/v1/customer.py +++ b/reboot/examples/bank-pydantic/api/bank/v1/customer.py @@ -36,16 +36,21 @@ class BalancesResponse(Model): request=None, response=None, factory=True, + description="Bring the customer into existence with no " + "accounts.", mcp=None, ), open_account=Transaction( request=OpenAccountRequest, response=OpenAccountResponse, + description="Open an account for this customer with an " + "initial deposit, returning the id it was given.", mcp=None, ), balances=Reader( request=None, response=BalancesResponse, + description="The balance of every account this customer owns.", mcp=None, ), ) @@ -54,5 +59,6 @@ class BalancesResponse(Model): Customer=Type( state=CustomerState, methods=CustomerMethods, + description="One customer, and the accounts they own.", ), ) diff --git a/reboot/examples/chick-potle/api/ai_chat_food/v1/food.py b/reboot/examples/chick-potle/api/ai_chat_food/v1/food.py index 02a45956..affe1ff0 100644 --- a/reboot/examples/chick-potle/api/ai_chat_food/v1/food.py +++ b/reboot/examples/chick-potle/api/ai_chat_food/v1/food.py @@ -110,6 +110,8 @@ class CreateOrderRequest(Model): request=CreateOrderRequest, response=None, factory=True, + description="Start the order. Called when " + "the customer begins, not by the AI.", mcp=None, ), get_menu=Reader( diff --git a/reboot/examples/reboot-swag-store/api/reboot_swag_store/v1/store.py b/reboot/examples/reboot-swag-store/api/reboot_swag_store/v1/store.py index 4688d845..239d8d6e 100644 --- a/reboot/examples/reboot-swag-store/api/reboot_swag_store/v1/store.py +++ b/reboot/examples/reboot-swag-store/api/reboot_swag_store/v1/store.py @@ -259,6 +259,8 @@ class GetDetailsResponse(Model): request=CartCreateRequest, response=None, factory=True, + description="Start an empty cart for one " + "shopper.", mcp=None, ), add_item=Writer( @@ -312,6 +314,8 @@ class GetDetailsResponse(Model): request=None, response=None, factory=True, + description="Bring the coupon book into " + "existence with no codes in it.", mcp=None, ), generate_codes=Writer( @@ -344,6 +348,8 @@ class GetDetailsResponse(Model): request=CreateOrderRequest, response=None, factory=True, + description="Place the order from a " + "checked-out cart.", mcp=None, ), fulfill=Workflow( diff --git a/tests/reboot/documentation/ai_chat_counter.py b/tests/reboot/documentation/ai_chat_counter.py index 0add64dc..ceb34273 100644 --- a/tests/reboot/documentation/ai_chat_counter.py +++ b/tests/reboot/documentation/ai_chat_counter.py @@ -100,6 +100,9 @@ class IncrementRequest(Model): request=InitializeCounterRequest, response=None, factory=True, + description="Create the counter at its " + "initial value. Called by " + "`User.create_counter`, not by the AI.", mcp=None, ), get=Reader( diff --git a/tests/reboot/documentation/chat_room_pydantic.py b/tests/reboot/documentation/chat_room_pydantic.py index 70bc7f6d..b36976f4 100644 --- a/tests/reboot/documentation/chat_room_pydantic.py +++ b/tests/reboot/documentation/chat_room_pydantic.py @@ -19,12 +19,14 @@ class SendRequest(Model): messages=Reader( request=None, response=MessagesResponse, + description="Every message posted to the room so far.", mcp=None, ), # Adds a new message to the list of recorded messages. send=Writer( request=SendRequest, response=None, + description="Post one message to the room.", mcp=None, ), ) diff --git a/tests/reboot/documentation/errors_pydantic.py b/tests/reboot/documentation/errors_pydantic.py index aafec4b4..7cba9e91 100644 --- a/tests/reboot/documentation/errors_pydantic.py +++ b/tests/reboot/documentation/errors_pydantic.py @@ -16,6 +16,8 @@ class OverdraftError(Model): request=WithdrawRequest, response=None, errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", mcp=None, ), # ... From d77cc7ed81bb5aabea0917e8134400e7611b54c9 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 18 Aug 2026 02:01:19 +0000 Subject: [PATCH 8/9] Update skills to take description in methods --- .../chat-app/references/api-method-types.md | 2 ++ reboot/plugin/skills/python/SKILL.md | 15 ++++++-- .../skills/python/references/api-errors.md | 2 ++ .../skills/python/references/api-methods.md | 32 +++++++++++++---- .../skills/python/references/api-pydantic.md | 36 ++++++++++++++++--- .../python/references/api-schema-evolution.md | 6 +++- .../references/patterns-error-handling.md | 2 ++ .../references/rpc-constructor-calls.md | 1 + .../python/references/scheduling-basic.md | 8 +++-- .../python/references/servicer-constructor.md | 1 + .../python/references/servicer-transaction.md | 2 ++ .../python/references/servicer-workflow.md | 4 +++ 12 files changed, 96 insertions(+), 15 deletions(-) diff --git a/reboot/plugin/skills/chat-app/references/api-method-types.md b/reboot/plugin/skills/chat-app/references/api-method-types.md index ce73b037..c54d3433 100644 --- a/reboot/plugin/skills/chat-app/references/api-method-types.md +++ b/reboot/plugin/skills/chat-app/references/api-method-types.md @@ -363,6 +363,8 @@ class DoPingPeriodicallyResponse(Model): do_ping_periodically=Workflow( request=DoPingPeriodicallyRequest, response=DoPingPeriodicallyResponse, + description="Ping on a fixed interval for as long as the " + "application runs.", # `Workflow` requires `mcp=` like every other factory; usually `None` # since workflows are rarely AI-callable tools directly. mcp=None, diff --git a/reboot/plugin/skills/python/SKILL.md b/reboot/plugin/skills/python/SKILL.md index c1946136..ca9bd256 100644 --- a/reboot/plugin/skills/python/SKILL.md +++ b/reboot/plugin/skills/python/SKILL.md @@ -159,9 +159,20 @@ api = API( ChatRoom=Type( state=ChatRoomState, methods=Methods( - messages=Reader(request=None, response=MessagesResponse, mcp=None), - send=Writer(request=SendRequest, response=None, mcp=None), + messages=Reader( + request=None, + response=MessagesResponse, + description="Every message posted so far, oldest first.", + mcp=None, + ), + send=Writer( + request=SendRequest, + response=None, + description="Post one message to the room.", + mcp=None, + ), ), + description="One chat room, and everyone posting into it.", ), ) ``` diff --git a/reboot/plugin/skills/python/references/api-errors.md b/reboot/plugin/skills/python/references/api-errors.md index e4891f03..249a5ca4 100644 --- a/reboot/plugin/skills/python/references/api-errors.md +++ b/reboot/plugin/skills/python/references/api-errors.md @@ -47,6 +47,8 @@ AccountMethods = Methods( request=WithdrawRequest, response=None, errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", mcp=None, ), # ... other methods ... diff --git a/reboot/plugin/skills/python/references/api-methods.md b/reboot/plugin/skills/python/references/api-methods.md index 7c19e626..c697baed 100644 --- a/reboot/plugin/skills/python/references/api-methods.md +++ b/reboot/plugin/skills/python/references/api-methods.md @@ -50,16 +50,23 @@ from reboot.api import ( AccountMethods = Methods( balance=Reader( - request=None, response=BalanceResponse, mcp=None, + request=None, response=BalanceResponse, + description="The funds currently available to withdraw.", + mcp=None, ), deposit=Writer( - request=DepositRequest, response=None, mcp=None, + request=DepositRequest, response=None, + description="Add funds. Any amount is accepted.", + mcp=None, ), ) BankMethods = Methods( transfer=Transaction( - request=TransferRequest, response=TransferResponse, mcp=None, + request=TransferRequest, response=TransferResponse, + description="Move funds between two accounts, both sides " + "landing together or neither.", + mcp=None, ), ) ``` @@ -90,10 +97,20 @@ snake_case. So ```python add_task=Transaction( - request=AddTaskRequest, response=AddTaskResponse, mcp=None, + request=AddTaskRequest, response=AddTaskResponse, + description="Append one task, returning the id it was given.", + mcp=None, +), +lists=Reader( + request=None, response=ListsResponse, + description="Every list this user owns.", + mcp=None, +), +ensure=Transaction( + request=None, response=None, + description="Create the user's default list if they have none.", + mcp=None, ), -lists=Reader(request=None, response=ListsResponse, mcp=None), -ensure=Transaction(request=None, response=None, mcp=None), ``` obliges exactly: @@ -134,6 +151,7 @@ open=Writer( request=OpenRequest, response=None, factory=True, + description="Bring the account into existence with a zero balance.", mcp=None, ), ``` @@ -171,6 +189,8 @@ withdraw=Writer( request=WithdrawRequest, response=None, errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", mcp=None, ), ``` diff --git a/reboot/plugin/skills/python/references/api-pydantic.md b/reboot/plugin/skills/python/references/api-pydantic.md index ced25254..40a5b128 100644 --- a/reboot/plugin/skills/python/references/api-pydantic.md +++ b/reboot/plugin/skills/python/references/api-pydantic.md @@ -72,28 +72,35 @@ AccountMethods = Methods( balance=Reader( request=None, response=BalanceResponse, + description="The funds currently available to withdraw.", mcp=None, ), deposit=Writer( request=DepositRequest, response=None, + description="Add funds. Any amount is accepted.", mcp=None, ), withdraw=Writer( request=WithdrawRequest, response=None, errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", mcp=None, ), open=Writer( request=None, response=None, factory=True, + description="Bring the account into existence with a zero " + "balance.", mcp=None, ), interest=Writer( request=None, response=None, + description="Credit one period's interest at the current rate.", mcp=None, ), ) @@ -103,6 +110,8 @@ api = API( Account=Type( state=AccountState, methods=AccountMethods, + description="One customer's money, and the consistency " + "boundary for every change to it.", ), ) ``` @@ -167,10 +176,28 @@ mcp ```python # All four factories — same shape: -balance=Reader(request=None, response=BalanceResponse, mcp=None), -deposit=Writer(request=DepositRequest, response=None, mcp=None), -transfer=Transaction(request=TransferRequest, response=None, mcp=None), -autoplay=Workflow(request=None, response=None, mcp=None), +balance=Reader( + request=None, response=BalanceResponse, + description="The funds currently available to withdraw.", + mcp=None, +), +deposit=Writer( + request=DepositRequest, response=None, + description="Add funds. Any amount is accepted.", + mcp=None, +), +transfer=Transaction( + request=TransferRequest, response=None, + description="Move funds between two accounts, both sides landing " + "together or neither.", + mcp=None, +), +autoplay=Workflow( + request=None, response=None, + description="Play the game out to a result without a human taking " + "turns.", + mcp=None, +), ``` Workflows in particular get caught by this — they're rarely @@ -370,6 +397,7 @@ api = API( create_checkers_game=Transaction( request=None, response=CreateCheckersGameResponse, + description="Start a checkers game, returning its id.", mcp=Tool(), ), ), diff --git a/reboot/plugin/skills/python/references/api-schema-evolution.md b/reboot/plugin/skills/python/references/api-schema-evolution.md index 18d64958..79527f54 100644 --- a/reboot/plugin/skills/python/references/api-schema-evolution.md +++ b/reboot/plugin/skills/python/references/api-schema-evolution.md @@ -119,12 +119,16 @@ api = API( # converts and delegates to the logic behind # `deposit_cents`. deposit=Writer( - request=DepositRequest, response=None, mcp=None, + request=DepositRequest, response=None, + description="Add funds, in dollars. Superseded by " + "`deposit_cents`.", + mcp=None, ), # New method with the corrected request shape # (`amount_cents: int`). deposit_cents=Writer( request=DepositCentsRequest, response=None, + description="Add funds, in whole cents.", mcp=None, ), ), diff --git a/reboot/plugin/skills/python/references/patterns-error-handling.md b/reboot/plugin/skills/python/references/patterns-error-handling.md index 6bc6a860..caa12075 100644 --- a/reboot/plugin/skills/python/references/patterns-error-handling.md +++ b/reboot/plugin/skills/python/references/patterns-error-handling.md @@ -39,6 +39,8 @@ withdraw=Writer( request=WithdrawRequest, response=None, errors=[OverdraftError], + description="Take funds out, or raise `OverdraftError` if the " + "balance would go negative.", mcp=None, ), ``` diff --git a/reboot/plugin/skills/python/references/rpc-constructor-calls.md b/reboot/plugin/skills/python/references/rpc-constructor-calls.md index c5339882..65e38713 100644 --- a/reboot/plugin/skills/python/references/rpc-constructor-calls.md +++ b/reboot/plugin/skills/python/references/rpc-constructor-calls.md @@ -38,6 +38,7 @@ open=Writer( request=OpenRequest, response=None, factory=True, + description="Bring the account into existence with a zero balance.", mcp=None, ), ``` diff --git a/reboot/plugin/skills/python/references/scheduling-basic.md b/reboot/plugin/skills/python/references/scheduling-basic.md index 2337c33f..1fe1d912 100644 --- a/reboot/plugin/skills/python/references/scheduling-basic.md +++ b/reboot/plugin/skills/python/references/scheduling-basic.md @@ -37,10 +37,14 @@ asyncio.create_task(self._fire_later()) # disappears on restart ```python open=Writer( - request=OpenRequest, response=None, factory=True, mcp=None, + request=OpenRequest, response=None, factory=True, + description="Bring the account into existence with a zero balance.", + mcp=None, ), interest=Writer( - request=None, response=None, mcp=None, + request=None, response=None, + description="Credit one period's interest at the current rate.", + mcp=None, ), ``` diff --git a/reboot/plugin/skills/python/references/servicer-constructor.md b/reboot/plugin/skills/python/references/servicer-constructor.md index 4dccbf48..46a53fa2 100644 --- a/reboot/plugin/skills/python/references/servicer-constructor.md +++ b/reboot/plugin/skills/python/references/servicer-constructor.md @@ -42,6 +42,7 @@ open=Writer( request=OpenRequest, response=None, factory=True, + description="Bring the account into existence with a zero balance.", mcp=None, ), ``` diff --git a/reboot/plugin/skills/python/references/servicer-transaction.md b/reboot/plugin/skills/python/references/servicer-transaction.md index 7b6a75d2..c0a28454 100644 --- a/reboot/plugin/skills/python/references/servicer-transaction.md +++ b/reboot/plugin/skills/python/references/servicer-transaction.md @@ -37,6 +37,8 @@ async def transfer( transfer=Transaction( request=TransferRequest, response=None, + description="Move funds between two accounts, both sides landing " + "together or neither.", mcp=None, ), ``` diff --git a/reboot/plugin/skills/python/references/servicer-workflow.md b/reboot/plugin/skills/python/references/servicer-workflow.md index 11881a13..1fc830b2 100644 --- a/reboot/plugin/skills/python/references/servicer-workflow.md +++ b/reboot/plugin/skills/python/references/servicer-workflow.md @@ -61,6 +61,8 @@ async def control_loop( control_loop=Workflow( request=ControlLoopRequest, response=None, + description="Answer messages for as long as the chatbot is " + "running.", mcp=None, ), ``` @@ -1373,6 +1375,8 @@ api = API( request=FulfillRequest, response=None, errors=[PaymentDeclined, CustomerSuspended], + description="Take the order from paid to shipped, " + "resuming where it left off after a restart.", mcp=None, ), ), From 0a7c6b113e23650cf980bce314210401e4339729 Mon Sep 17 00:00:00 2001 From: Riley Scheid Date: Tue, 18 Aug 2026 02:01:44 +0000 Subject: [PATCH 9/9] Fix README to take required mcp and optional description args --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index a035c154..fca5c820 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,7 @@ api = API( request=None, response=CreateCounterResponse, description="Create a new Counter.", + mcp=Tool(), ), ), ), @@ -84,6 +85,8 @@ api = API( request=None, response=None, factory=True, + description="Create the counter at zero.", + mcp=None, ), get=Reader( request=None,