docs(plugin): bring boundary text in line with the implementation

Comments that pointed at design notes kept outside the repository are dead
ends for anyone reading only the tree, so what each of them explained now
lives in a tagged TODO at the site it applies to. Several of those sites
also stated something the implementation does not do, and the TODOs record
the actual gap instead: the two halves of the codec recognize handle
descriptors by different rules, the scripts Command path drops the exception
class and collects output in a BufferedOutput that cannot carry an
interactive command, find_shortest_path panics where PHP throws, and the
package dispatch hand-rolls the variant selection AnyPackage should own.

The classifier document likewise described rust-snapshot,
plugin-constructible and several of the open questions as designed rather
than as built.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

3 files changed, 47 insertions, 14 deletions

diff --git a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs
index fb3ad5f4..a5a238bd 100644
--- a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs
+++ b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs
@@ -751,11 +751,12 @@ impl EventDispatcher {
 
                         // PHP hosts the user's Command class in a throwaway, bare
                         // `Symfony\Component\Console\Application` (NOT Composer's Application),
-                        // built by a generated snippet running inside the worker. The command's
-                        // output is captured in a BufferedOutput and written back through the
-                        // dispatcher's IO; upstream hands the live output object of `$this->io`
-                        // to `$app->run()` instead, so only the interleaving with concurrent
-                        // writes differs.
+                        // built by a generated snippet running inside the worker.
+                        //
+                        // TODO(plugin): the BufferedOutput has to go. The command has to run
+                        // against the real output stream, the way upstream hands the live output
+                        // object of `$this->io` to `$app->run()`; collecting the output and
+                        // writing the buffer back once the run has returned is not a substitute.
                         let args = additional_args
                             .iter()
                             .map(|arg| ProcessExecutor::escape(arg))
@@ -775,6 +776,12 @@ impl EventDispatcher {
                         } else {
                             output_interface::VERBOSITY_NORMAL
                         };
+                        // TODO(error-model): the snippet's try/catch does not reproduce upstream's
+                        // boundary. Upstream wraps `$app->run()` alone and catches `\Exception`, so
+                        // an `\Error` from the command, and a throw from `new $className(...)`,
+                        // both escape without the "terminated with an exception" line. Here the
+                        // catch is `\Throwable`, and a constructor throw leaves the snippet as a
+                        // `Throw` reply from `__shirabe_eval`, so the line is written either way.
                         let snippet = format!(
                             r#"
 $className = {class_name_lit};
@@ -821,6 +828,8 @@ try {{
                         )?;
                         let result = match outcome {
                             Ok(value) => value.to_php_mixed()?,
+                            // TODO(error-model): `throw.exception_class` is dropped, so the class
+                            // upstream rethrows unchanged collapses to RuntimeException here.
                             Err(throw) => {
                                 self.io.write_error3(
                                     &format!(
@@ -846,6 +855,9 @@ try {{
                             self.io.write3(&command_output, false, crate::io::NORMAL);
                         }
                         if let Some(throw) = result.as_array().and_then(|map| map.get("throw")) {
+                            // TODO(error-model): the snippet reports `get_class($e)` as the first
+                            // field and nothing reads it, so the class upstream rethrows unchanged
+                            // collapses to RuntimeException here.
                             let fields = throw
                                 .as_list()
                                 .expect("the eval snippet reports exceptions as a list");
diff --git a/crates/shirabe/src/plugin/php_plugin_proxy.rs b/crates/shirabe/src/plugin/php_plugin_proxy.rs
index c9d6986b..8469593f 100644
--- a/crates/shirabe/src/plugin/php_plugin_proxy.rs
+++ b/crates/shirabe/src/plugin/php_plugin_proxy.rs
@@ -732,9 +732,10 @@ fn dispatch_config_method(
 
 /// The download manager's contract is asynchronous on both sides: PHP declares a
 /// `PromiseInterface` return, Rust an `async fn`. The Rust future is driven to completion here
-/// and its value handed back as an already-settled React promise, which is the synchronous
-/// fallback of the promise design (`.ken/plugin-arch/design.md` §10.1.6) rather than the
-/// deferred resolution a concurrent engine would allow.
+/// and its value handed back as an already-settled React promise.
+///
+/// TODO(async): the boundary has no representation for a promise that is still pending, so the
+/// deferred resolution the PHP contract allows collapses into a blocking wait here.
 fn dispatch_download_manager_method(
     dm: &std::rc::Rc<std::cell::RefCell<dyn crate::downloader::DownloadManagerInterface>>,
     method_name: &str,
@@ -1006,12 +1007,15 @@ fn dispatch_event_dispatcher_method(
             if dispatcher.borrow_mut().has_event_listeners(&probe) {
                 // TODO(plugin): dispatching a worker-constructed event through the Rust-side
                 // dispatcher needs the event object (and the console input it carries) proxied
-                // back into this process; until then only the no-listener case — where
-                // upstream's dispatch is observably a no-op returning 0 — is supported.
+                // back into this process, so only the no-listener case is answered here.
                 return Err(runtime_throw(format!(
                     "dispatching `{name}` from the plugin process is not supported yet while listeners are registered for it"
                 )));
             }
+            // TODO(plugin): answering 0 here skips what `do_dispatch` does before it reaches the
+            // listener loop, and upstream does both regardless of the listener count: the
+            // `COMPOSER_DEBUG_EVENTS` trace line, and `push_event`'s circular-call detection
+            // (a nested dispatch of the same event name throws there even with no listeners).
             Ok(PluginValue::Int(0))
         }
         other => Err(runtime_throw(format!(
@@ -1610,6 +1614,15 @@ fn dispatch_package_method(
         }
         // `RootAliasPackage` overrides each of these to write through to its alias target, and
         // `RootPackage` reaches the same base state either way, so both go through the interface.
+        //
+        // TODO(type-model): choosing the body for the concrete variant belongs on `AnyPackage`,
+        // not here. The stub surface already decides which classes carry a method, so the
+        // `as_*_mut` accessors' "not available on an alias package" arms are unreachable for a
+        // method the alias stubs do not declare, and what is left is a per-variant dispatch that
+        // this guard only approximates: it covers `RootPackage` as well, where the extra hop is
+        // equivalent only while that impl keeps delegating to the base package, and nothing
+        // checks it. Method names repeated in the base-package arm below make the answer depend
+        // on arm order, and a new variant compiles into the wrong body without a diagnostic.
         "setRequires" | "setDevRequires" | "setConflicts" | "setProvides" | "setReplaces"
         | "setAutoload" | "setDevAutoload" | "setSuggests" | "setExtra"
             if package.borrow().is_root() =>
@@ -2497,8 +2510,10 @@ impl PhpInstallerProxy {
     /// The `?PromiseInterface` half of the installer contract. The Rust callers await the
     /// installer's effects rather than chaining continuations, so a returned promise is drained
     /// here: an already-settled one yields its value (or raises its rejection reason), while a
-    /// still-pending one is an explicit error — resolving it would need the concurrent execution
-    /// engine the boundary does not have (`.ken/plugin-arch/design.md` §10.1.6).
+    /// still-pending one is an explicit error.
+    ///
+    /// TODO(async): resolving a still-pending promise would need a concurrent execution engine
+    /// the boundary does not have.
     fn promise_result(&self, method: &str, value: PluginValue) -> anyhow::Result<Option<PhpMixed>> {
         let handle = match value {
             PluginValue::Null => return Ok(None),
diff --git a/crates/shirabe/src/util/filesystem.rs b/crates/shirabe/src/util/filesystem.rs
index 8408a8a9..b20f5f48 100644
--- a/crates/shirabe/src/util/filesystem.rs
+++ b/crates/shirabe/src/util/filesystem.rs
@@ -533,8 +533,12 @@ impl Filesystem {
         prefer_relative: bool,
     ) -> String {
         if !self.is_absolute_path(from) || !self.is_absolute_path(to) {
-            // PHP throws InvalidArgumentException
-            // Returning early-formatted Result is not possible without changing signature; panic to surface in tests.
+            // TODO(error-model): PHP throws InvalidArgumentException. Plugins reach this method
+            // through the RPC proxy, where a relative path is ordinary input rather than a
+            // programming error, and this panic kills the process instead of reaching their
+            // catch block; the plugin dispatcher repeats the check for that reason. Returning
+            // `anyhow::Result` from here and from `find_shortest_path_code` removes both the
+            // panic and the duplicated check.
             panic!(
                 "{}",
                 format!("$from ({}) and $to ({}) must be absolute paths.", from, to)
@@ -596,6 +600,8 @@ impl Filesystem {
         prefer_relative: bool,
     ) -> String {
         if !self.is_absolute_path(from) || !self.is_absolute_path(to) {
+            // TODO(error-model): as in `find_shortest_path` — PHP throws
+            // InvalidArgumentException, and this panic cannot reach a plugin's catch block.
             panic!(
                 "{}",
                 format!("$from ({}) and $to ({}) must be absolute paths.", from, to)