📝 Clarify execution contracts and repair compiled PR review

.reviews/ReviewPR.local.md

@@ -23,11 +23,7 @@ const changedTsFiles = pr.files
   as="rawDiagnostics"
  />
 
-```ts eval
-import { buildDiagnostics } from "@executablemd/code-review-agent";
-
-const diagnostics = buildDiagnostics(rawDiagnostics, pr, doctor);
-```
+<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />
 
 <ReleaseSpecWarning files={changedFilePaths} />
 

.reviews/ReviewPR.md

@@ -25,11 +25,7 @@ const changedTsFiles = pr.files
   as="rawDiagnostics"
  />
 
-```ts eval
-import { buildDiagnostics } from "@executablemd/code-review-agent";
-
-const diagnostics = buildDiagnostics(rawDiagnostics, pr, doctor);
-```
+<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />
 
 <ThinkFilter>
 <DeepInfraProvider model="Qwen/Qwen3-30B-A3B">

AGENTS.md

@@ -405,7 +405,9 @@ the corpus or enabling in-shard concurrency is not an answer to a miss.
 ## MUST READ
 
 - https://github.com/thefrontside/effection/blob/v4/AGENTS.md
-- `architecture.md` — read fully
+- [`architecture.md`](architecture.md) — read fully. Its [reading map](architecture.md#reading-this-architecture)
+  locates current invariants and their feature-interface owners; it does not
+  replace the full reading.
 - `specs/executable-mdx-spec.md` — read the sections your change touches
 
 ## Code Rules

README.md

@@ -22,6 +22,21 @@ Three documents own the rest:
 - [`specs/executable-mdx-spec.md`](specs/executable-mdx-spec.md) — what the
   product does, with the conformance matrix that pins it.
 
+Follow a question into the [architecture reading map](architecture.md#reading-this-architecture):
+
+- [Execution and diagnostic journals](architecture.md#a-document-running): what
+  runs, what a Plan produces, and which journal can continue a workflow.
+- [Partial and completed replay](architecture.md#replay-and-effect-transactions):
+  why a historical file read survives a later deletion.
+- [Native Agent ownership](architecture.md#native-agent-sessions): handoff,
+  release and executable compatibility.
+- [Installed structural regions](architecture.md#the-installed-structural-syntax-boundary):
+  demand, output and cancellation.
+- [Plan recovery](architecture.md#typed-candidate-recovery-around-evaluate): why
+  cleanup failure stops a candidate retry.
+- [Current rules and historical decisions](specs/decisions.md#finding-the-current-rule):
+  where the architecture, feature contracts and vision differ in authority.
+
 Locate the package that owns the behavior you are about to change:
 
 - `packages/core/` — the engine: the boundary scanner, component expansion,
@@ -33,7 +48,7 @@ Locate the package that owns the behavior you are about to change:
 - `packages/durable-streams/` — the journal protocol, replay and divergence.
 - `packages/workflow/` — workflow runs, run storage, and the Workspace.
 - `packages/git/` — repository collaboration: `<Repository>`, `<Worktree>`,
-  `<Dir>`, the `Git.*` operations, pull requests and issues, and the GitHub
+  the `Git.*` operations, pull requests and issues, and the GitHub
   adapter behind them. It is the one Plugin `xmd` bundles, active by default for
   the run-profile commands — `run`, `plan`, `syntax`, and `workflow start`,
   `resume` and `fork`. The `xmd test` root is deliberately not one of them, so a

packages/cli/src/plan-writer-profile.ts

@@ -22,8 +22,8 @@
  * with nothing in it. `--approve-all`, `--approve-reads` and `--deny-all`
  * configure the approved document, later, and reach none of this.
  *
- * The document itself is given no Files, command, service or XMD-mediated
- * network capability either. It decides what to write; it writes nothing.
+ * The trusted document reads through the ordinary Files provider and effective
+ * Permissions. This frame refuses Files mutations, Process, Service and Fetch.
  */
 
 import { ensure, Err, Ok, scoped, until, useScope } from "effection";

packages/cli/tests/plan-component.test.ts

@@ -767,17 +767,11 @@ describe("Tier PC — <Plan> in an ordinary document", () => {
     /**
      * Cancelling an information read, with its cleanup held open.
      *
-     * The grant is live exactly across `tryContent()` and the teardown the
-     * projection joins, so the interesting interruption is one that lands
-     * *inside* the provider: the read is in flight, a cleanup registered with it
-     * holds, and the halt may not be allowed to complete until that cleanup
-     * finishes. `joined` read while the cleanup is still held is what proves it
-     * waited — a flag checked afterwards would be true either way.
-     *
-     * What it shows about access is what a cancelled run can show: nothing
-     * further is performed under the grant. The retained-guard case — a guard
-     * answering no once its frame has ended — is Tier PLA's, where a scope can
-     * outlive what it was asking about.
+     * The interruption lands inside the provider: the read is in flight and
+     * its registered cleanup is held open. The projection joins that teardown,
+     * so halting the run must wait until the cleanup finishes. Reading `joined`
+     * before releasing the cleanup proves the halt waited — a flag checked
+     * afterwards would be true either way.
      */
     it("PI27: cancelling an entered information read waits for its cleanup", function* () {
       yield* useWorkingDirectory(function* () {

packages/code-review-agent/src/components.md

@@ -1,19 +1,19 @@
 Long-form documentation for the code-review agent's TypeScript components.
 
-Six components that gather the evidence a review reasons about: the pull request
-and its diff, the checkout's readiness, the Oxlint sensor's diagnostics, the
+Seven components gather or group the evidence a review reasons about: the pull
+request and its diff, the checkout's readiness, the Oxlint sensor's diagnostics, the
 repository's source inventory, and the review comments already on the pull
-request. Each one *reads* — none of them writes a comment, pushes a branch or
-changes a file.
+request. They read or transform evidence without writing a comment, pushing a
+branch or changing a file.
 
-They are reserved: this package claims these six names, so a repository under
+They are reserved: this package claims these seven names, so a repository under
 review cannot supply its own `ReviewContext.ts` and have a review run it. The
 Markdown components that compose them are declared by this package for the same
 reason, and are documented by their own bodies rather than here.
 
-Every one of them runs against the working directory. A review of another
-checkout establishes that checkout first — with `<Worktree>` or `<Dir>` — and
-these report on whatever is current.
+The evidence-gathering components run against the working directory. A review
+of another checkout establishes that checkout first — with `<Worktree>` or
+`<Dir>` — and these report on whatever is current.
 
 ## ReviewContext
 
@@ -67,6 +67,18 @@ cannot use. A silent empty result would be indistinguishable from a clean run,
 and a review that reported "no findings" because its sensor died is worse than
 one that stopped.
 
+## ReviewDiagnostics
+
+Group normalized Oxlint diagnostics for review policies.
+
+```mdx
+<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />
+```
+
+Groups the sensor results by rule, applies the Doctor's import-noise filtering,
+and computes categories, density and summary through the package's existing
+`buildDiagnostics` helper. This transformation uses only its supplied values.
+
 ## RepositoryInventory
 
 Counts what is in the repository, for a review that reasons about the whole tree

packages/code-review-agent/src/components/ReviewDiagnostics.ts

@@ -0,0 +1,68 @@
+import type { Operation } from "effection";
+import type { Diagnostics, DoctorResult, OxlintDiagnostic, PR } from "../types.ts";
+import { buildDiagnostics } from "../parse-diagnostics.ts";
+import { returns as rawDiagnosticsSchema } from "./OxlintDiagnostics.ts";
+import { returns as doctorSchema } from "./Doctor.ts";
+
+export const props = {
+  type: "object",
+  properties: {
+    raw: rawDiagnosticsSchema,
+    pr: { type: "object" },
+    doctor: doctorSchema,
+  },
+  required: ["raw", "pr", "doctor"],
+  additionalProperties: false,
+};
+
+const groupSchema = {
+  type: "object",
+  properties: {
+    ruleId: { type: "string" },
+    count: { type: "number" },
+    files: { type: "array", items: { type: "string" } },
+    instances: rawDiagnosticsSchema,
+  },
+  required: ["ruleId", "count", "files", "instances"],
+  additionalProperties: false,
+};
+const groupsSchema = { type: "array", items: groupSchema };
+
+export const returns = {
+  type: "object",
+  properties: {
+    groups: groupsSchema,
+    total: { type: "number" },
+    fileCount: { type: "number" },
+    ruleCount: { type: "number" },
+    byCategory: {
+      type: "object",
+      properties: {
+        structural: groupsSchema,
+        verbosity: groupsSchema,
+        typeAware: groupsSchema,
+        other: groupsSchema,
+      },
+      required: ["structural", "verbosity", "typeAware", "other"],
+      additionalProperties: false,
+    },
+    summary: { type: "string" },
+    density: { type: "number" },
+  },
+  required: ["groups", "total", "fileCount", "ruleCount", "byCategory", "summary", "density"],
+  additionalProperties: false,
+};
+
+interface ReviewDiagnosticsProps {
+  raw: OxlintDiagnostic[];
+  pr: PR;
+  doctor: DoctorResult;
+}
+
+export default function* ReviewDiagnostics({
+  raw,
+  pr,
+  doctor,
+}: ReviewDiagnosticsProps): Operation<Diagnostics> {
+  return buildDiagnostics(raw, pr, doctor);
+}

packages/code-review-agent/src/plugin.ts

@@ -2,7 +2,7 @@
  * Install the packaged review graph when an operator selects this Plugin.
  *
  * Run, syntax, plan and executing workflow actions reserve its thirty-five
- * Markdown declarations and six reserved registrations, so the repository being
+ * Markdown declarations and seven reserved registrations, so the repository being
  * reviewed cannot replace the review program. Root `xmd test` reserves none;
  * its isolated `<Execution host="run">` child reinstalls this Plugin with
  * command `run`.
@@ -100,7 +100,7 @@ const reviewPlugin: Plugin = Plugin({
     if (!claimsGraph(request)) {
       return undefined;
     }
-    // The six reserved registrations and the documentation describing them, in
+    // The seven reserved registrations and the documentation describing them, in
     // the scope the command runs in. Declarative only: nothing here installs a
     // provider, spawns a process, reads a credential or reaches the network.
     yield* useReviewComponents();

packages/code-review-agent/src/review-components.ts

@@ -2,7 +2,7 @@
  * Declare the packaged review graph before a root document is imported.
  *
  * Thirty-five Markdown components cross as {@link MarkdownComponent} values
- * with packaged bytes, stable asset origins and digests. Six TypeScript
+ * with packaged bytes, stable asset origins and digests. Seven TypeScript
  * components use reserved registrations. Both reserve names so the repository
  * being reviewed cannot replace its reviewers; duplicate host reservations
  * refuse at admission. Ordinary includes and component discovery remain
@@ -51,6 +51,10 @@ import RepositoryInventory, {
   props as repositoryInventoryProps,
   returns as repositoryInventoryReturns,
 } from "./components/RepositoryInventory.ts";
+import ReviewDiagnostics, {
+  props as reviewDiagnosticsProps,
+  returns as reviewDiagnosticsReturns,
+} from "./components/ReviewDiagnostics.ts";
 import ReviewContext, {
   props as reviewContextProps,
   returns as reviewContextReturns,
@@ -174,7 +178,7 @@ function reviewComponent(fn: (props: any) => Operation<unknown>): FunctionCompon
 }
 
 /**
- * Reserve the six TypeScript component names alongside the Markdown declarations.
+ * Reserve the seven TypeScript component names alongside the Markdown declarations.
  * These implementations run processes, read credentials and reach the network;
  * leaving them shadowable would let the review subject replace its own reviewers.
  * Their existing public names and contracts are retained.
@@ -244,6 +248,21 @@ export const REVIEW_REGISTRATIONS: readonly ComponentRegistration[] = [
       context: null,
     }),
   },
+  {
+    name: "ReviewDiagnostics",
+    origin: REVIEW_ORIGIN,
+    reserved: true,
+    props: reviewDiagnosticsProps,
+    returns: reviewDiagnosticsReturns,
+    fn: reviewComponent(ReviewDiagnostics),
+    ...documented({
+      description:
+        `Group normalized Oxlint diagnostics for review policies. ` +
+        `\`<ReviewDiagnostics raw={rawDiagnostics} pr={pr} doctor={doctor} as="diagnostics" />\`.`,
+      as: "The grouped diagnostics, categories, density, and review summary.",
+      context: null,
+    }),
+  },
   {
     name: "RepositoryInventory",
     origin: REVIEW_ORIGIN,
@@ -303,7 +322,7 @@ export function* reviewDocumentation(
 }
 
 /**
- * Install the six reserved registrations and the documentation describing them.
+ * Install the seven reserved registrations and the documentation describing them.
  *
  * One call, both halves, for the reason every other package bootstrap is one
  * call: a scope that has the components has the words that describe them, and

packages/core/src/component-answers.ts

@@ -14,12 +14,10 @@
  * handler invocation then receives its own {@link ComponentAnswerRequest},
  * which is the right to answer *that* asking and nothing else.
  *
- * The split is the whole point. A stable handle a provider retains can only
- * ever prove "some provider installed by this host" — it cannot prove which
- * invocation is speaking, so a handler that has already returned, or one that
- * lost, could state an answer to an import it is not deciding. A request proves
- * it, because it is minted for one invocation, captures the resolution it was
- * asked in by object identity, and is closed the moment that invocation ends.
+ * An installation is reusable, but cannot identify which invocation is
+ * speaking. Each request captures its resolution by object identity and closes
+ * when its handler returns, throws or is cancelled, so a finished or losing
+ * handler cannot state an answer to an import it is no longer deciding.
  *
  * So `claim` takes no name. The name is fixed when the request is minted, from
  * what the chain asked, and a handler cannot state an answer for a component it
@@ -31,11 +29,11 @@
  *
  * ## Identifying is not permission
  *
- * An identity statement permits nothing. A fragment still needs the host entry, an exact identity match, the right `allow`
- * class, whole- fragment preflight and canonical generated import — all of
- * them independently. Middleware that answers an import without stating an
- * identity is an ordinary replacement and stays exactly as valid as it was;
- * what it cannot do is enter an admitted fragment.
+ * An identity statement permits nothing. A fragment still needs the host entry,
+ * an exact identity match, the right `allow` class, whole-fragment preflight
+ * and canonical generated import — all of them independently. Middleware that
+ * answers an import without stating an identity is an ordinary replacement
+ * and stays exactly as valid as it was; it cannot enter an admitted fragment.
  */
 
 import type { Operation } from "effection";
@@ -67,9 +65,8 @@ export type ComponentAnswerHandler = (
 /**
  * What a provider installs through, in place of anything it could keep.
  *
- * Registering composes the middleware; canonical execution owns the request
- * lifetime around each invocation of it, so a provider cannot hold the lease
- * open past its own handler.
+ * Registering composes import middleware without handing the provider a
+ * request it can retain to answer a future invocation.
  */
 export interface ComponentAnswerRegistrar {
   /**
@@ -113,11 +110,6 @@ export function componentAnswerRegistrar(
     *around(handler: ComponentAnswerHandler): Operation<void> {
       yield* Component.around({
         *importComponent([name, position], next) {
-          // One request for this handler invocation, capturing the resolution
-          // window that is open right now. Minted here rather than carried on
-          // the installation: one installation serves every name it registered,
-          // and a retained handle states nothing about which resolution is
-          // being decided.
           const asked = installation.open(name, position);
           try {
             return yield* handler(
@@ -126,10 +118,6 @@ export function componentAnswerRegistrar(
                 next(forName ?? name, forPosition ?? position),
             );
           } finally {
-            // Synchronous, and in `finally`, so the lease ends the same way the
-            // invocation did — returned, threw, or was cancelled. Nothing
-            // yields here: a close that needed a suspension point would leave
-            // the request open across one.
             asked.close();
           }
         },

packages/core/src/components/component-resolution.ts

@@ -245,10 +245,11 @@ interface Claim {
   /**
    * One retained copy per resolution this answer was identified in.
    *
-   * Weak on the window, so an identity record lasts exactly as long as
-   * somebody still holds the resolution it belongs to. Presence is the
-   * provenance check: a window that never identified this answer has no entry,
-   * and `identify` answers nothing for it however many other windows did.
+   * Window keys are weak: retaining an answer does not keep its past
+   * resolutions alive. The outer record is weakly keyed by the answer, too.
+   * Presence is the provenance check: a window that never identified this
+   * answer has no entry, and `identify` answers nothing for it however many
+   * other windows did. Neither weak association promises collection timing.
    */
   readonly copies: WeakMap<ResolutionWindow, ImportedDefinition | undefined>;
 }
@@ -548,11 +549,10 @@ export class CanonicalImports {
     const held =
       typeof answer === "object" && answer !== null ? this.#claims.get(answer) : undefined;
     if (held !== undefined) {
-      // What this object is was settled by its first claim. Restating it is what
-      // a provider installed twice does, and what a reusable one does for every
-      // import it answers; anything else is two providers disagreeing about one
-      // object, and the first statement stands — a later claim that overwrote it
-      // would let a second provider rename the first's implementation.
+      // The same installation may restate this object's identity for another
+      // import or repeat it within one resolution. A different installation
+      // cannot restate it, even identically: the first provider's statement
+      // stands, so a second cannot rename the first's implementation.
       if (
         held.installation !== installation ||
         held.name !== asked.name ||