Cut comments back to the non-obvious

Rationale that explains a decision rather than the code below it belongs in
the sandbox README or the PR, not in a header paragraph on every file.

packages/platform/src/web/connection.ts

@@ -50,14 +50,7 @@ export class WorkerConnection {
   /** True once the worker has said anything at all. */
   private heard = false;
 
-  /**
-   * Who answers a template function, once something can.
-   *
-   * The engine renders in the worker but the functions come from plugins in
-   * this tab's sandbox, so the worker asks back through this. Unset until the
-   * sandbox is up, and a render that arrives before then gets the same refusal
-   * a host with no plugins gives — which is the truth at that moment.
-   */
+  /** Unset until the sandbox is up; a render before then gets a refusal. */
   private templateFunctions: ((name: string, args: string) => Promise<string>) | null = null;
 
   constructor() {
@@ -172,7 +165,6 @@ export class WorkerConnection {
     });
   }
 
-  /** Hand the worker somewhere to send template functions. */
   setTemplateFunctionHandler(handler: (name: string, args: string) => Promise<string>): void {
     this.templateFunctions = handler;
   }

packages/platform/src/web/index.ts

@@ -58,9 +58,8 @@ function capabilitiesFor(): PlatformCapabilities {
     // The browser draws the frame around the page. There are no traffic lights
     // to leave room for and no window controls to draw.
     windowChrome: false,
-    // Plugins run here, in a QuickJS sandbox — see packages/plugin-sandbox.
-    // What is missing is installing them: the set is the one bundled with the
-    // app, so the plugin *manager* stays unavailable and says so.
+    // Plugins run in a QuickJS sandbox, but only the bundled set: there is no
+    // installing them, so the plugin manager stays unavailable and says so.
     plugins: true,
     encryption: false,
     updater: false,
@@ -164,9 +163,7 @@ export function createWebPlatform(): Platform {
   const plugins = new WebPlugins(db);
   const capabilities = capabilitiesFor();
 
-  // Rendering happens in the worker and template functions live in the sandbox,
-  // so the worker needs a way back here to call one. Registered before anything
-  // can render, which is why it is here rather than inside the first send.
+  // Registered before anything can render, not inside the first send.
   db.setTemplateFunctionHandler((name, args) => plugins.callTemplateFunction(name, args));
 
   // Without this, IndexedDB is best-effort storage and a browser reclaiming

packages/platform/src/web/plugins.ts

@@ -1,17 +1,7 @@
 /**
- * The plugins this host runs, and everything they are allowed to reach.
- *
- * Two jobs. Outward: keep a sandbox, load the bundled plugins into it, and know
- * which of them answers what — the app asks for "the bearer auth config" and
- * this decides that means `auth-bearer`. Inward: answer the `ctx` calls those
- * plugins make, which is where the sandbox stops being a sealed box and starts
- * being a host. Everything a plugin can do to the world is in `hostRequest`
- * below, by name, with a refusal for anything not listed.
- *
- * The plugins are bundled into the app rather than installed, for now — see
- * `scripts/bundle-sandbox-plugins.mjs`. Which three, and why only three, is a
- * decision that belongs to this slice and not to the sandbox: the runtime does
- * not know how many plugins exist.
+ * Keeps a sandbox, loads the bundled plugins into it, routes by what each one
+ * contributes, and answers the `ctx` calls they make. `hostRequest` below is
+ * the whole of what a plugin can do to the world here.
  */
 
 import { PluginSandbox, type PluginSummary } from "@yaakapp-internal/plugin-sandbox";
@@ -28,7 +18,6 @@ import type {
 import type { WorkerConnection } from "./connection";
 import { SANDBOX_PLUGINS } from "./sandboxPlugins.generated";
 
-/** What a plugin's own storage is keyed under, matching the desktop's namespacing. */
 type KeyValueRequest = { key: string };
 
 export interface AppliedAuthentication {
@@ -41,7 +30,6 @@ export class WebPlugins {
   private sandbox: PluginSandbox | null = null;
   private loading: Promise<void> | null = null;
 
-  /** Loaded plugin ids, by what they contribute. */
   private readonly byTemplateFunction = new Map<string, string>();
   private readonly byAuthName = new Map<string, string>();
   private readonly importers: string[] = [];
@@ -52,12 +40,8 @@ export class WebPlugins {
   }
 
   /**
-   * Bring the sandbox up and load every bundled plugin, once.
-   *
    * Called from every entry point rather than at construction, so a session
-   * that never touches a plugin never pays for QuickJS — and so a tab that
-   * cannot start the worker still boots the app, with plugin-shaped features
-   * failing individually instead of the page failing entirely.
+   * that never touches a plugin never pays for QuickJS.
    */
   ready(): Promise<void> {
     this.loading ??= this.start();
@@ -68,16 +52,13 @@ export class WebPlugins {
     const sandbox = new PluginSandbox({
       onHostRequest: (envelope) => this.hostRequest(envelope),
       onLog: ({ pluginRefId, level, message }) => {
-        // Prefixed, because otherwise a plugin's console output is
-        // indistinguishable from the app's own and blames the wrong code.
+        // Prefixed, or a plugin's console output blames the app's own code.
         const write = level === "error" ? console.error : console.log;
         write(`[plugin ${pluginRefId}] ${message}`);
       },
     });
     this.sandbox = sandbox;
 
-    // In parallel: each is an independent QuickJS context and none of them
-    // observes the others.
     await Promise.all(
       SANDBOX_PLUGINS.map(async ({ name, source }) => {
         try {
@@ -106,13 +87,7 @@ export class WebPlugins {
     return this.gather("get_http_authentication_summary_request", this.byAuthName.values());
   }
 
-  /**
-   * Ask several plugins the same question and keep the answers that came back.
-   *
-   * A plugin that has nothing to say answers `empty_response`, which is not an
-   * answer and is dropped; one that throws is logged and dropped too, so a
-   * single broken plugin cannot empty the picker for all the others.
-   */
+  /** One broken plugin must not empty the picker for the others. */
   private async gather<T>(type: string, ids: Iterable<string>): Promise<T[]> {
     const replies = await Promise.all(
       Array.from(ids).map(async (id): Promise<{ type: string } | null> => {
@@ -146,13 +121,9 @@ export class WebPlugins {
   }
 
   /**
-   * Run one template function.
-   *
-   * This is what the engine's render calls back into, so its contract is the
-   * engine's: a string, or a throw whose message says what went wrong. A
-   * function nothing provides is a throw naming it rather than an empty
-   * string, because a request sent with a silently blank token is worse than
-   * one that refuses to be sent.
+   * What the engine's render calls back into. A function nothing provides is a
+   * throw naming it, not an empty string: a request sent with a silently blank
+   * token is worse than one that refuses to be sent.
    */
   async callTemplateFunction(name: string, argsJson: string): Promise<string> {
     await this.ready();
@@ -204,13 +175,7 @@ export class WebPlugins {
     } as InternalEventPayload);
   }
 
-  /**
-   * Apply an authentication method to a request that is about to be sent.
-   *
-   * The plugin is shown the request as it stands and hands back headers and
-   * query parameters to add — the same exchange the desktop has, at the same
-   * point in the send.
-   */
+
   async applyHttpAuthentication(
     authName: string,
     request: {
@@ -235,13 +200,7 @@ export class WebPlugins {
     } as InternalEventPayload);
   }
 
-  /**
-   * Import whatever this text turns out to be.
-   *
-   * Every importer is asked and the first one that recognizes it wins, which
-   * is how the desktop's `import_data` decides too — an importer that does not
-   * recognize its input returns nothing rather than guessing.
-   */
+  /** First importer that recognizes the text wins, as `import_data` decides too. */
   async import(content: string): Promise<ImportResources | null> {
     await this.ready();
     for (const id of this.importers) {
@@ -269,26 +228,16 @@ export class WebPlugins {
   }
 
   /**
-   * The context a plugin sees.
-   *
-   * `label` is null and stays null: it names a desktop window, and the calls
-   * that need one — `ctx.window.requestId()` and its neighbours — are refused
-   * rather than answered with a guess about which request the user is looking
-   * at. `workspaceId` is genuinely unknown here for the same reason; the
-   * commands that know it pass it themselves.
+   * `label` names a desktop window, so it stays null and the calls needing one
+   * refuse rather than guess which request the user is looking at.
    */
   private context(): PluginContext {
     return { id: "web", label: null, workspaceId: null };
   }
 
   /**
-   * Answer one `ctx` call.
-   *
-   * The list is short on purpose. What is here is what a plugin can do in a
-   * browser tab today; what is missing refuses by name, so a plugin that needs
-   * it fails with a sentence someone can act on rather than a hang or an
-   * undefined. Every addition to this list is a capability decision, which is
-   * why they are written out one at a time instead of forwarded wholesale.
+   * Every addition here is a capability decision, which is why they are written
+   * out one at a time instead of forwarded wholesale.
    */
   private async hostRequest(envelope: string): Promise<string> {
     const { pluginRefId, payload } = JSON.parse(envelope) as {
@@ -299,7 +248,6 @@ export class WebPlugins {
 
     const reply = async (): Promise<InternalEventPayload> => {
       switch (payload.type) {
-        /* A plugin's own storage, namespaced by plugin in the database. */
         case "get_key_value_request": {
           const value = await this.db.rpc<string | null>("web_plugin_kv_get", {
             pluginName: pluginRefId,
@@ -324,7 +272,6 @@ export class WebPlugins {
           return { type: "delete_key_value_response", deleted } as InternalEventPayload;
         }
 
-        /* A message for the user, delivered where every other one is. */
         case "show_toast_request": {
           const { type: _type, ...toast } = payload;
           this.db.deliver("show_toast", toast);

packages/platform/src/web/protocol.ts

@@ -16,10 +16,7 @@ export type ToWorker =
    * async in the engine (rendering is), where every `rpc` command is not.
    */
   | { type: "prepare_http_send"; id: number; payload: unknown }
-  /**
-   * Render one template string. Async for the same reason `prepare_http_send`
-   * is: a template function is a call out to a plugin, and plugins are not here.
-   */
+  /** Async for the same reason `prepare_http_send` is: it can call a plugin. */
   | { type: "render_template"; id: number; payload: unknown }
   /** The tab's answer to a `template_function` call. */
   | {
@@ -51,14 +48,7 @@ export type FromWorker =
   | { type: "error"; id: number; message: string }
   /** A backend event for the app — today only `model_writes`. Sent to every port. */
   | { type: "event"; event: string; payload: unknown }
-  /**
-   * Render a template function, please.
-   *
-   * The one message that runs the other way. Rendering happens in the engine,
-   * here, but the functions it calls live in a plugin sandbox the tab owns —
-   * so the engine asks, and it asks the port that started the render rather
-   * than broadcasting, because only that tab is waiting.
-   */
+  /** The one message that runs the other way: the engine asking for a plugin. */
   | { type: "template_function"; id: number; name: string; args: string };
 
 /** What the worker registers itself under. Tabs on one origin share it. */

packages/platform/src/web/send.ts

@@ -52,7 +52,7 @@ type ResponsePatch = Partial<HttpResponse>;
 /** What `prepare_http_send` (crates/yaak-web) hands back. */
 interface PreparedHttpSend {
   request: HttpRequest;
-  /** Hashed id of the model the authentication came from; a plugin keys its state on it. */
+  /** Hashed id of the model the auth came from; plugins key stored state on it. */
   authContextId: string;
   settings: HttpSendSettings;
   settingEvents: HttpResponseEventData[];
@@ -60,20 +60,13 @@ interface PreparedHttpSend {
 }
 
 /**
- * Apply the request's authentication method, if it has one.
+ * The desktop applies auth to the sendable request; here the proxy builds that,
+ * so the plugin's answer goes onto the model and the proxy folds it in. Same
+ * bytes for a method that sets a header, which is every one that runs here.
  *
- * The desktop does this to the request it is about to put on the wire, after
- * rendering and after building the sendable form of it. Here the sendable form
- * is built by the proxy, so the plugin's answer is applied to the model instead
- * — headers onto `headers`, query parameters onto `urlParameters` — and the
- * proxy folds both in exactly as it would any others. The result on the wire is
- * the same for a method that sets a header, which is every method whose plugin
- * runs in the browser today.
- *
- * It is not the same for a method that *signs* the request, because the plugin
- * is shown the request before the proxy assembles it: AWS SigV4 and OAuth 1.0
- * would sign a URL and a header set slightly different from the ones sent. Both
- * are refused rather than silently mis-signed — see the sandbox README.
+ * Not the same for one that *signs*, since the plugin sees the request before
+ * the proxy assembles it. AWS SigV4 and OAuth 1.0 are refused rather than
+ * mis-signed; see the sandbox README.
  */
 async function applyAuthentication(
   plugins: WebPlugins,
@@ -90,17 +83,14 @@ async function applyAuthentication(
     method: request.method,
     url: request.url,
     headers: request.headers.filter((h) => h.enabled !== false),
-    // The desktop passes the body so signing schemes can hash it. This host
-    // does not have it in bytes at this point, and the schemes that would use
-    // it are the ones already refused.
+    // Only signing schemes hash the body, and those are already refused.
     body: null,
   });
 
   const headers = [...request.headers];
   for (const header of applied.setHeaders ?? []) {
     // Replace-or-append, case-insensitively, matching `insert_header` in
-    // crates/yaak-http: a plugin setting Authorization must not end up with the
-    // request's own Authorization also on the wire.
+    // crates/yaak-http.
     const at = headers.findIndex((h) => h.name.toLowerCase() === header.name.toLowerCase());
     const entry = { name: header.name, value: header.value, enabled: true };
     if (at >= 0) headers[at] = { ...headers[at], ...entry };

packages/platform/src/web/worker.ts

@@ -109,17 +109,9 @@ function bootOnce(): Promise<void> {
 }
 
 /**
- * Template functions, which live somewhere this worker cannot reach.
- *
- * Rendering is the engine's, and the engine is here. The functions it calls
- * come from plugins, which run in a sandbox the tab owns — so the engine is
- * handed a function that asks the tab. It asks the port that started the
- * render, not every port, because only that tab is waiting on the answer and
- * only its sandbox has the plugins the render was started against.
- *
- * A failure comes back as a rejection, which the engine turns into a render
- * error naming the function. That matters: rendering `${[ uuid.v4() ]}` to an
- * empty string and sending it would be worse than not sending at all.
+ * Rendering happens here; the functions it calls live in a sandbox the tab
+ * owns. Asked of the port that started the render, not every port, because
+ * only that tab is waiting and only its sandbox has those plugins.
  */
 const pendingTemplateFunctions = new Map<number, (result: string | Error) => void>();
 let nextTemplateFunctionId = 1;