Skip to content

design(http): HTTP client/controller as a metered pass-style adapter pipeline - #992

Draft
kriscendobot wants to merge 8 commits into
endojs:llmfrom
kriscendobot:design-http-adapter-pipeline
Draft

design(http): HTTP client/controller as a metered pass-style adapter pipeline#992
kriscendobot wants to merge 8 commits into
endojs:llmfrom
kriscendobot:design-http-adapter-pipeline

Conversation

@kriscendobot

Copy link
Copy Markdown
Collaborator

Design follow-up: HTTP client/controller as a metered pass-style adapter pipeline

Follow-up to the approval of endo http mk Phase 1 in
#286 (review).
This is a design-only PR — one new design document plus index/cross-link updates. No production code.

What it does

Elaborates the endo http controller/client pair (designs/cli-http-client.md) into a composable pass-style adapter pipeline that stages five cross-cutting concerns as exo-facet middleware — metering, fees, rate limiting, retries, and circuit breaking (error-based) — mining the Koa/axios/undici adapter-pipeline prior art the review pointed to.

Key design moves

  • Middleware onion → pass-style facets. Each stage is an exo implementing a uniform HttpStageInterface whose request() calls E(next).request(...), with next captured at composition on the controller side. A stage is substitutable for the whole client to the stage above it, so the chain can be arbitrarily deep and partly remote (a fee purse held in the gateway vat participates over CapTP). Preserves the Phase 1 invariant: the controller holds immutable policy; the client only exercises it — a guest cannot insert, reorder, or reconfigure a stage.
  • Metering and the Phase 1 byte cap become one mechanism. maxResponseBytes doubles as the worst-case response term of an up-front reservation; the bounded read that enforces the cap is exactly what bounds actual cost ≤ reserved cost. Aligned to the minion.town gateway metering ground rules: reserve worst-case before headers, refuse in the pessimal case before reading any bytes, settle delivered bytes plus wall-clock capped at the deadline; measurement happens at the resource boundary, not the caller (a security property that falls out of pass-style for free). Reuses the daemon-xs-worker-metering admission-control / budget-as-pre-payment model.
  • Fees through an attenuated charge account. The fee stage draws against an ERTP-style makeChargeAccount(purse, { limit, expiresAt, … }), endowed controller-side and never client-facing (the analogue of "the base @endo/fetch is never guest-facing"). A compromised meter stage can drain at most the account's limit. Refusal surfaces as a structured InsufficientFunds rejection before the network is touched; a pure estimateCost probe lets a guest pre-check affordability without authority.
  • Canonical stage order. Pure pre-flight (origin/method/header, zero side effects) + effectful onion breaker › retry › rate › meter › transport, with the rationale for each position (retries billed and throttled per attempt; a tripped origin or rate refusal costs no funds; idempotent-only retries with deadline-shared jittered backoff; per-origin breaker keyed on server/transport error classes, not 4xx).
  • Staged into the existing phase plan (Phase 3 substrate, 3.5 metering+fees, 3.6 rate-aggregate+retries, 4 breaker) and preserves the SSRF/DoS posture (redirect: 'manual', host-curated allowlist, read-time truncation).

Files

  • designs/http-adapter-pipeline.md — new design.
  • designs/cli-http-client.md — forward pointer from § Out of scope, future work.
  • designs/README.md — index entries.

Opening as draft for design review.

🤖 Generated with Claude Code

…pipeline

Elaborate the endo http controller/client pair (cli-http-client.md,
Phase 1 approved in PR endojs#286) into a composable pass-style adapter
pipeline staging five cross-cutting concerns as exo-facet stages:
metering, fees, rate limiting, retries, and circuit breaking.

- Maps the Koa/axios/undici middleware onion onto pass-style facets:
  each stage is an exo whose request() calls E(next).request(), with
  next captured at composition on the controller side, preserving the
  Phase 1 invariant (controller holds immutable policy, client only
  exercises).
- Reconciles metering with the Phase 1 byte cap as ONE mechanism:
  maxResponseBytes doubles as the worst-case response term of an
  up-front reservation. Aligns with the minion.town gateway metering
  ground rules (reserve worst-case before headers, refuse in the
  pessimal case before reading bytes, settle delivered-bytes plus
  wall-clock capped at the deadline; measurement at the resource
  boundary, not the caller) and the daemon-xs-worker admission-control
  model. Fees thread through an attenuated ERTP charge account endowed
  controller-side, never client-facing.
- Defines the canonical stage order (pure pre-flight + effectful onion:
  breaker > retry > rate > meter > transport), the new controller verbs
  and endo http CLI verbs, and stages the work into cli-http-client's
  Phase 3/3.5/3.6/4 plan. Preserves the SSRF/DoS posture.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel disposition: must-fix (posted as a comment review; GitHub disallows request-changes on one's own PR).

Jury panel verdict — round 1: must-fix

Design panel reviewed the PR diff (3 files; designs/http-adapter-pipeline.md, designs/cli-http-client.md, designs/README.md) against the PR base. All seats returned request-changes. Aggregated per-seat findings follow.

critic

critic

Verdict: request-changes

Findings:

  • The canonical stage order (§ "Canonical stage order" table: breaker=1/outermost … retry=2 … rate=3 … meter=4 … transport=5/innermost, composed "transport first, then meter, rate, retry, breaker around it") is internally inconsistent with § "5. Circuit breaking → Position and interaction with retry," which claims the breaker "wraps retry, so it observes every attempt … and can trip mid-retry-loop," and that "once the breaker opens, the retry stage's next E(next).request fast-rejects with CircuitOpenError." Under the document's own composition model (§ "The crucial adaptation": "Each stage exo is built endowed with the far-ref to the stage beneath it," captured once at construction, not passed per call), if breaker is outermost its next is retry — called exactly once per top-level request. Retry's own next (per the stated order) is the rate limiter, three hops away from the breaker; retry's internal per-attempt loop never touches the breaker at all. So the breaker cannot observe individual attempts or trip mid-loop as claimed — it only ever sees the single aggregated outcome of the whole retry sequence. The narrative's own claim ("the retry stage's next … fast-rejects with CircuitOpenError") only makes sense if breaker is retry's own next — i.e., breaker sits inside retry (between retry and rate), the reverse of the table. This is a foundational contradiction in the design's flagship "desired coupling" (retry storms tripping the breaker, which then halts the storm) — as specified, that coupling does not work, and a builder has no way to know which of the two contradictory statements is authoritative. Fix: either state the stage order as retry(outer) → breaker → rate → meter → transport (which preserves "breaker gates before any resource-spending stage" and gives breaker per-attempt visibility), or, if breaker must stay strictly outermost, specify an explicit out-of-band consultation seam the retry stage uses to read/report to breaker state directly. [proposed-rule: a multi-stage pass-style pipeline design must include a worked trace (prose or diagram) of at least one multi-attempt scenario showing exactly which stage calls which next, so ordering claims in prose are checked against the stated composition mechanics before merge.]
  • Minor, related: the RequestContextShape code sketch (§ "The stage interface") declares only origin, attempt, deadline, and reservation as fields, but the surrounding prose says ctx "far-refs … carry effectful shared state a stage may consult" with meter and breaker both cited as examples. No breaker-state far-ref appears in the shape. This is consistent with the ordering contradiction above (there is no channel by which retry could consult breaker state) and should be resolved together with it rather than left as a sketch gap. [proposed-rule: same as above.]

Notes (out of scope but worth flagging):

  • A five-stage onion with this many order-dependent invariants (rate above meter, meter's ceiling reused as reservation, breaker/retry coupling) is exactly the shape that benefits from a mermaid sequence diagram of one successful request and one retried-then-tripped request; it likely would have surfaced the ordering contradiction above during authoring. [rule: roles/jurors/critic/AGENT.md § Operating norms, diagrams-use-mermaid]

Self-improvement: none — the brief's guidance matched the review surface encountered; no gaps to report.

skeptic

skeptic

Verdict: request-changes

Findings:

  • The stage-order table contradicts the circuit breaker's own per-attempt claim. § Canonical stage order composes the onion outer→inner as breaker(1) → retry(2) → rate(3) → meter(4) → transport(5), and the composition model (§ Prior art, "Each stage exo is built endowed with the far-ref to the stage beneath it") means a stage's only visibility is the single call/response that crosses it via next. Retry's next is the rate limiter, not the breaker — the breaker sits above retry, so it is invoked exactly once per top-level request, wrapping the entire retry loop as one call/response. Yet § Circuit breaking claims the breaker "wraps retry so it sees every attempt and can trip mid-loop," and § Retries claims each attempt is "visible to … the breaker, which records every attempt." Given the stated compose-at-construction, far-ref-only model, the breaker structurally cannot observe individual attempts inside a retry loop it wraps — it only sees the loop's final outcome. Either the breaker must sit inside retry (between retry and rate), or the design needs to admit an out-of-band side-channel from retry to breaker state, which would itself contradict the "no stage mutates another's fields" invariant asserted for RequestContext. This is load-bearing: the fixer cannot implement both the stated order and the stated per-attempt breaker behavior as written. [proposed-rule: a design describing a pass-style stage pipeline must verify that any claim about a stage's per-call observability is consistent with its position in the compose order — a stage sees only what its own next reference passes through it.]

  • 429 is counted as breaker/retry "origin unhealthy" evidence, undermining the design's own guest-fairness rationale for excluding 4xx. § Circuit breaking states the breaker excludes 4xx because "the request was wrong, the origin is fine" and explicitly reasons that "counting client-fault errors would let a guest trip the breaker for every other guest by sending garbage" — yet the same section lists "5xx, 429, connection refused, …" as counted evidence. 429 (Too Many Requests) is itself a 4xx status and is frequently triggered by one guest's own request volume, not origin health. Per the breaker's own per-origin-shared-across-guests scope ("two guests of the same controller hitting the same dead origin share the evidence"), a single greedy or misbehaving guest tripping upstream 429s would trip the breaker for every other guest of that controller on that origin — precisely the cross-guest failure mode the 4xx exclusion was designed to prevent. The retry stage has the identical inconsistency ("5xx / 429 responses" retried, "does not retry 4xx"). [proposed-rule: a design's stated exclusion criterion (here, "client-fault errors do not count toward shared state") must be checked against every status code it enumerates as an exception, not asserted and then silently violated by a same-section example.]

Notes (out of scope but worth flagging):

  • Reusing maxResponseBytes as both DoS truncation ceiling and reservation ceiling (§ Metering) changes its operational meaning for existing Phase 1 deployments — a generously-sized cap tuned only for truncation now directly inflates cost_max and can cause legitimate low-balance requests to be refused pessimally. Not flagged as a migration/re-tuning concern. [proposed-rule: a design that overloads an existing policy knob with a new cost semantic should call out the retuning existing deployments need.]
  • Test plan omits a concurrency test for the claimed atomic reserve-serialization ("concurrent reservations … cannot jointly exceed limit") and omits any test for estimateCost. [rule: designs/AGENTS.md § Test plan completeness expectation via panel-hints]

decomplector

decomplector

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:122 claims RequestContext is an immutable record and states "no stage mutates another's fields," but the design's own prose repeatedly describes it as mutated in place: line 150's shape comment reads reservation: M.remotable('MeterReservation'), // set by the meter stage, and line 479 says "Each attempt increments ctx.attempt." "Set by" and "increments" are place-oriented verbs applied to a structure the design bills as a value. Endo's own pass-style discipline (every record crossing CapTP must be harden()ed, hence non-mutable) means the substance is almost certainly "each stage forwards a freshly-constructed ctx with that field updated," but the document's language braids the value-oriented threading model with a place-oriented mutation model without reconciling them — exactly the state-with-identity-with-time complecting this seat watches for (attempt and reservation are both time-indexed values being described as if they were one mutable place). An implementer following the prose as written risks building ctx as a literal mutable object, which would then need a rewrite once the hardening requirement surfaces. Should-fix: state explicitly that each hop constructs a new hardened RequestContext (e.g. { ...ctx, attempt: ctx.attempt + 1 }) rather than "setting"/"incrementing" the existing one. [rule: roles/jurors/decomplector/AGENT.md § Operating norms (d) value-oriented vs place-oriented]

Notes (out of scope but worth flagging):

  • The write-side controller surface is five bespoke setters (setMeterPrice, attachChargeAccount, setControllerMaxRequestsPerMinute, setRetry, setBreaker, designs/http-adapter-pipeline.md:534-538), one per concern, while the read side already found the uniform primitive (inspectPipeline(), line 539, one call covering every stage's live state regardless of type). The design itself names a concrete future stage (TOFU pin, § Relationship to existing designs) that will need a sixth bespoke setter under the current shape. This is a familiar (easy) shape — mirrors Phase 1's per-knob setters — but the design's own observation #2 ("optional and per-deployment... from the same controller vocabulary") suggests the smaller unifying primitive (a uniform configureStage(name, patch) verb, symmetric with the already-uniform read side) was reachable but not pursued. Not a blocker for this proposal; worth a Hammock-driven second pass before the Phase 3/3.5 verbs are frozen. [rule: roles/jurors/decomplector/AGENT.md § Operating norms (f) minimum viable abstraction]

Self-improvement: none — the panel-review citation convention and the decomplector's operating-norms categories both worked as documented for this review; no gap surfaced.

ergonomist

ergonomist

Verdict: request-changes

Findings:

  • RequestContextShape (designs/http-adapter-pipeline.md:192-198) is declared as a single shape used identically for two different callers with very different trust levels: the client-facing request(req, cancellation, ctx) entry point a guest may call directly, and the internal stage-to-stage accumulator each onion layer threads. The shape includes reservation: M.remotable('MeterReservation') as a plain optional field. This contradicts the prose a few paragraphs later ("the RequestContext the client may pass carries only a caller-supplied deadline proposal … it cannot carry a stage or a purse") — the invariant lives only in prose, not in the type the reader is shown, so a builder implementing straight from the interface sketch has no self-evident signal that reservation must never be accepted on the client boundary. Split into two shapes (e.g. CallerContextShape with only deadline, vs. an internal StageContextShape extending it with reservation/attempt), or annotate the shared shape to mark internal-only fields. [rule: roles/jurors/ergonomist/AGENT.md § Operating norms (naming for the user's mental model / affordance)]

  • Return-guard idiom is inconsistent across sibling methods on the same interfaces: HttpStageInterface.request and ChargeAccountInterface.reserve both declare .returns(M.promise()) (an unchecked escape hatch), while sibling methods HttpStageInterface.help and ChargeAccountInterface.getBalance declare a checked resolved-value guard (M.string(), M.number()) for calls that are equally remote/async. Nothing in the design explains why request/reserve opt out of return-shape checking while their siblings opt in — "do similar operations spell similarly" is violated at the interface-guard level, not just the naming level. Either guard request/reserve against their known resolved shapes (ResponseShape, MeterReservationInterface) or state why they're deliberately unchecked. [rule: roles/jurors/ergonomist/AGENT.md § Operating norms (return-shape coherence across siblings)]

  • MeterReservationInterface.settle is explicitly documented idempotent, keyed on measurementId; the sibling release has no key and no idempotency statement, leaving retry-safety of the abort path undefined next to a carefully-specified settle path. [proposed-rule: sibling capability-consuming methods on one interface document matching idempotency/retry-safety guarantees, or state why they differ]

Notes (out of scope but worth flagging):

  • CLI verb set-price drops the "meter" qualifier its siblings (set-retry, set-breaker, set-rate) each keep for their concern name; harmless but slightly breaks scan-ability of the verb table. [proposed-rule: CLI subcommand names should retain the concern name their sibling verbs use]
  • setMaxRequestsPerMinute (Phase 1, per-client) sits unprefixed next to the new setControllerMaxRequestsPerMinute (per-controller); the existing name doesn't self-disclose scope, which will read as an odd pair once both exist. Compatibility-driven, flagging for the namer dispatch rather than blocking here.

Self-improvement: no update proposed to roles/jurors/ergonomist/AGENT.md this round — the brief's existing "return-shape coherence across siblings" and "naming for the caller's mental model" norms covered every finding without a gap.

copyeditor

copyeditor

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:19-21, 225, 232, 284-286, 315-317, 321, 333, 466, 668 — the document is heavy with typist-hostile code points outside any exempt context: prose arrows (, e.g. "rate take() → method/header normalization → origin check → ... → byte-cap", "outer → inner", "Meter (reserve → settle)"), the middle-dot multiplication sign (·) used repeatedly as an operator inside the cost_max/cost_actual fenced formulas and in `base · 2^attempt`, the minus sign in `cost_max − cost_actual`, and in `cost_actual ≤ cost_max` / `bytesActuallyRead ≤ maxResponseBytes`. None of these fall under the skill's exemptions (not a verbatim upstream quote, not fenced output, not a code span quoting a lone glyph — each code span/formula mixes the glyph with surrounding text). Replace with ->, *, -, and <= throughout. [rule: skills/typist-friendly-code-points/SKILL.md]

  • designs/http-adapter-pipeline.md:235-238 — "http-confine spends the rate token (step 1) before the origin check (step 3)" back-references ordinal positions in the six-step chain from the Problem section (line 19-21), but that chain is only ever written as an arrow-linked prose list, never enumerated with step numbers. A reader has to count arrows in an earlier paragraph to confirm "step 1" = rate take() and "step 3" = origin check. Either number the original chain (1. rate take() 2. ... 3. origin check ...) or spell the steps out by name here instead of by ordinal. [proposed-rule: an ordinal back-reference ("step N") to an earlier list must point at a list that is itself numbered, not an unnumbered arrow chain]

Notes (out of scope but worth flagging):

  • None.

Self-improvement: none — the brief and cited skills covered this review without gaps.

pedant

Now I have enough to write the review.

Verdict: request-changes

Findings:

  1. Must-fix — pervasive em-dash use throughout the new design document. designs/http-adapter-pipeline.md contains 56 em-dashes (, U+2014) in prose (e.g. line 15 "shipped its defenses — origin allowlist", line 350 heading "### 2. Fees — the purse capability", line 622 "this design solves —"), and the added paragraph in designs/README.md line 10 ("Phase 1 approval — elaborates the") adds one more. [rule: skills/em-dash-style/SKILL.md] — rewrite each as a period, parentheses, or colon per the skill's guidance (designs/cli-http-client.md's hunk in this same diff does this correctly with zero em-dashes, so the pattern to follow is right next to the violation). This is the project's canonical style override and the volume here (every section, most subheadings) reads as unedited draft prose, not a shipped document.

  2. Must-fix — hard-to-type code points throughout. Right-arrow (lines 19-20, 108, 225, 232, 295, 493-495), ellipsis (lines 47, 94, 108, 354, 608), minus sign (line 321), and (lines 333, 668) all appear in prose, tables, and code-block comments. [rule: skills/typist-friendly-code-points/SKILL.md] — mechanically substitute ->, ..., -, <= respectively; none of these sit inside a code span quoting the glyph itself (the exemption), they're content.

  3. Should-fix — inconsistent heading case and terminal punctuation. Every ##/### heading in the new document is sentence case except ## What is the Problem Being Solved? and ## Alternatives Considered, which are title case; separately, the four ### Alt A/B/C/D: subheadings (lines 606, 617, 629, 640) end in a full stop while every other heading in the document does not. [rule: roles/jurors/pedant/AGENT.md § Operating norms, heading capitalization consistency] — pick sentence case (the document's dominant convention) and drop the terminal periods on the Alt headings.

Self-improvement: none — the brief's coverage of em-dash and typist-code-point rules was sufficient to catch both classes of violation without ambiguity.

novice

Now writing the review block.

novice

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:245-350 (## The five concerns) numbers the concerns metering(1)/fees(2)/rate(3)/retries(4)/breaking(5), but the reader just built a different mental model from ## Canonical stage order (line 207), whose table orders execution breaker→retry→rate→meter→transport. The two numbered lists name the same five things in near-opposite order and the document never says why (it silently follows the prompt's listing order, given only at line 782, far below). A reader trying to hold "the pipeline order" in mind loses the thread exactly where the design gets specific. [proposed-rule: a design document that presents the same ordered set twice (an execution-order table and a per-item detail list) must either use matching numbering or explicitly say the second list is not in pipeline order.]
  • Same span: "the five concerns" (metering, fees, rate, retries, breaking) implies five pipeline stages, but the stage-order table (line ~213-232) has only four non-transport rows — breaker, retry, rate, meter — with no row for fees. The reader has to infer, unaided, that fees isn't a stage at all but funding for the meter stage; the document never states that mapping directly. [rule: roles/jurors/novice/AGENT.md § Operating norms (skipped steps in reasoning)]
  • designs/http-adapter-pipeline.md:236 ("http-confine spends the rate token (step 1) before the origin check (step 3)") back-references an ordered list from the Problem section (~line 15, "rate take() → method/header normalization → origin check → …") that was never itself numbered. By the time the reader reaches line 236, ~220 lines have passed; counting "rate=1st, origin=3rd" from an unnumbered prose list that far back is a real break in top-down flow. [rule: roles/jurors/novice/AGENT.md § Operating norms (logical progress / skipped steps)]
  • designs/http-adapter-pipeline.md:66-67 uses the term "minion.town gateway metering ground rules" in ## Scope, before the reader has any idea what minion.town is — that context (a sibling garden repo, not in this tree) only arrives at the Relationship table, ~75 lines later. [rule: roles/jurors/novice/AGENT.md § Secondary overlap (mental-model gap after jargon is technically introduced)]

Notes (out of scope but worth flagging):

  • designs/http-adapter-pipeline.md:560 references "Phase 1/2" but Phase 2's content is never described anywhere in this document (only Phase 1, 3, 4 get content). Minor, since this is a follow-up to cli-http-client.md where Phase 2 is presumably defined. [proposed-rule: an elaboration document should not introduce an unexplained phase number even in passing, if it names phases the reader hasn't seen defined.]

Self-improvement: none — the brief's operating norms covered this review without gaps.

model claude-opus-4-8 · harness claude · garden 4e95097a

Address the design panel's request-changes on the HTTP adapter pipeline:

- Circuit-breaker ordering (critic, skeptic): the stage-order table had
  the breaker strictly outermost, contradicting its per-attempt
  observation claims. Under the compose-at-construction, far-ref-only
  model an outermost breaker sees only the whole retry loop's aggregate
  outcome. Reorder to retry -> breaker -> rate -> meter -> transport, so
  the breaker (retry's next) is invoked once per attempt: it both gates
  before any resource-spending stage and observes every attempt, and a
  mid-loop trip fast-rejects the next attempt. Table, composer order,
  and the retry/breaker/rate prose all updated to match.
- 429 as breaker evidence (skeptic): 429 reflects request volume, not
  origin health, so counting it lets one greedy guest trip the shared
  per-origin breaker for co-guests. Exclude 429 from breaker evidence;
  keep it retryable (Retry-After floor) and document the deliberate
  retry-vs-breaker asymmetry.
- RequestContext value-orientation (decomplector): drop "set by"/
  "increments" mutation language; each hop forwards a freshly hardened
  { ...ctx, attempt: ctx.attempt + 1 } record.
- Shape split + guards + idempotency (ergonomist): split the shared ctx
  into CallerContextShape (deadline only) vs internal StageContextShape;
  explain why request/reserve return unguarded while help/getBalance
  guard their bare primitives; document release() idempotency.
- Typist/em-dash sweep (copyeditor, pedant): remove all em-dashes and
  hard-to-type code points (arrows, middot, minus, <=, ellipsis) from
  the new doc and the one added README line; drop terminal periods on
  the Alt A/B/C/D headings.
- Reader mental-model gaps (novice): note that "The five concerns" is
  not in pipeline order and that fees is not a stage; gloss minion.town
  at first mention; replace the "step 1/step 3" ordinal back-reference
  with named steps.

Also add test-plan cases for concurrent-reservation serialization and
estimateCost, which the panel flagged as omitted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel verdict (round 2): must-fix

Design panel (7 seats) reviewed PR #992 against base 9d86783c0. Disposition: must-fix — five seats requested changes with concrete findings; two were comment-only.

Seat Verdict
critic request-changes
skeptic request-changes
decomplector request-changes
ergonomist request-changes
copyeditor comment-only
pedant comment-only
novice request-changes
Full per-seat findings

critic

critic

Verdict: request-changes

Findings:

  • [should-fix] The stage-context boundary is under-specified and the one guard shown contradicts the surrounding prose. CallerContextShape ({ deadline? }) is declared as "what the client boundary accepts," but HttpStageInterface.request's third argument is guarded by StageContextShape alone, whose first (required) M.splitRecord positional argument mandates origin: M.string() and attempt: M.number(). The doc states request has "the same signature as HttpClientInterface.request," so the client's own call is guarded by the same schema — meaning a guest's request(req, cancellation, { deadline }) call, as described, would fail its own method guard for lacking origin/attempt. CallerContextShape is defined but never referenced by any guard in the sketch. The document never shows the seam where a caller's { deadline } proposal becomes the origin (computed by the pure pre-flight, which runs before the onion) plus attempt: 0 that the outermost effectful stage requires — precisely the security-relevant boundary the design is making capability claims about ("a guest cannot supply attempt or reservation"). Fix: either give the client-facing surface its own, looser guard (M.or(CallerContextShape, StageContextShape) or a distinct HttpClientInterface), or sketch the adapter that synthesizes the initial StageContext. [proposed-rule: an illustrative interface sketch in a design doc must not have its own prose claims (e.g. "the client cannot supply X") contradicted by the very guard shown for the same method.]

  • [should-fix] estimateCost(req) is named seven times as the client's "pure, side-effect-free" probe but is absent from HttpStageInterface (only request and help). Since cost_max depends on the meter stage's private price policy, and the composition-opacity invariant says a guest "cannot even enumerate [stages] beyond what allowedOrigins()-style inspection exposes," the design doesn't show how a thin-forwarder client reaches an interior stage's pricing without either handing it a direct meter reference (breaking opacity) or adding a new pass-through method to HttpStageInterface that every intervening stage (retry/breaker/rate) must also implement — itself an open design question (does an open breaker affect estimateCost's answer?). [proposed-rule: same as above — a named client capability referenced repeatedly must appear in the interface it's claimed to belong to.]

Notes (out of scope but worth flagging):

  • This PR's designs/README.md edit adds only a designs/ list entry and a summary-table row; designs/AGENTS.md requires new designs to also land a milestone assignment, a milestone-table row, a dependency-graph node, and a size/duration estimate. Likely a pedant/spec-keeper item. [rule: designs/AGENTS.md § Progress Tracking § Cross-document]

Self-improvement: none this round — the brief and cite-or-propose discipline were sufficient for a design-only diff of this size.

skeptic

Result: skeptic review of PR #992 (kriscendobot/endo-but-for-bots)

skeptic

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:322-331 — the metering cost formula's load-bearing claim ("every term is known before headers are sent or a single response byte is read") is false for the perByteRequest term. The doc states len(request.body) comes "from the request ReadableBlob's length" (line 328), but RequestShape.body is typed M.remotable('ReadableBlob') (designs/cli-http-client.md:213) and ReadableBlobInterface's actual method set (packages/platform/src/fs/interfaces.js, readableBlobMethodGuards) is { help, streamBase64, text, json }, all returns(M.promise()) consuming methods, with no length/size accessor at all. Computing len(request.body) synchronously is not possible without either draining the body (defeating the "before any byte is read" guarantee this design's whole reserve-before-read mechanism rests on) or adding a field this design never proposes. An implementer cannot build the reserve step as specified. Must-fix: either add a plain contentLength/size field to RequestShape alongside body, or explicitly downgrade the request-body term to "estimated/capped, not known" and say what bounds it. [rule: designs/cli-http-client.md § HttpClientInterface]

  • Test plan gap: the design makes a specific capability-boundary security claim — "the client boundary accepts only the narrower CallerContextShape, so a guest cannot supply attempt or reservation" (lines 218-219) — but the Test plan (§ Test plan) has no bullet exercising it (no case asserting a client-supplied attempt/reservation field is stripped or rejected at the boundary). Every other prose security claim in this doc (rate-before-meter, breaker-before-rate, boundary measurement) has a matching test bullet; this one doesn't. [proposed-rule: a design's test plan must include a case for every "cannot supply X" capability-boundary claim made in its own body, since an untested boundary claim is unverifiable]

  • Comment-only: the doc says the pipeline is "a deliberate generalization of http-confine's fixed order" with "two substantive changes" (pre-flight-before-rate, breaker position), but a third, undisclosed reordering exists: http-confine runs method+header validation before origin (designs/http-confine.md § Rate Accounting: "method validation, header validation, origin validation"), while the new pre-flight runs origin before method+header (lines 285-289). Low-impact (both still precede the onion) but worth naming as a third change rather than leaving it implicit. [rule: designs/http-confine.md § Rate Accounting]

Self-improvement: none this round; the review procedure and brief matched the task cleanly.

decomplector

decomplector

Verdict: request-changes

Findings:

  • operationId is asked to do two incompatible identity jobs at once, and the document never reconciles them. ChargeAccountInterface.reserve is "keyed by operationId for idempotency" (designs/http-adapter-pipeline.md:350) — a replay-safety key, where the same operationId on a retried call must return the same hold rather than double-reserve. But the retry-stage section states "a 3-attempt request reserves and settles three times (each with its own worst case)" (designs/http-adapter-pipeline.md:541-544) — which requires three distinct operationIds. The design never states the derivation that keeps these from colliding (e.g. requestId:attempt), so a builder can satisfy either claim but not both from the interface as written: reuse one id per request and attempt 2 silently reuses attempt 1's reservation (violates "reserves three times"), or mint a fresh id per attempt and CapTP message redelivery for the same attempt double-reserves (violates "idempotency"). This is a state-with-identity conflation: one field is standing in for two different identity concepts (message-replay dedup vs. per-attempt billing distinctness) with no rule distinguishing them. [rule: roles/jurors/decomplector/AGENT.md § Operating norms, complecting check (a)]

  • The meter's reservation far-ref is threaded into StageContextShape — the type forwarded to every stage below the meter — with no described consumer other than the meter stage itself. Point 4 of the design's own motivating observations states state has "different scopes" and "a flat struct cannot express these scopes" (designs/http-adapter-pipeline.md:50-52), yet the settle description (designs/http-adapter-pipeline.md:356-359) has the meter stage settle using the reservation it itself created in step 1, never reading it back out of ctx. Putting the live capability in the generically-forwarded context (rather than the meter stage's own closure state, captured at composition like next) widens its reachability to the transport stage below for no described reason, and to any future stage inserted between meter and transport by mistake. Either drop reservation from StageContextShape and settle from the meter's held reference, or name the downstream consumer that needs it. [proposed-rule: a field added to a pass-style stage's shared/forwarded context record must name the specific downstream consumer that reads it, or it belongs in the originating stage's own closure state instead]

Notes (out of scope but worth flagging):

  • The deadline-clamping vs. cost_max interaction (does a caller-shortened deadline actually lower the reserved worst case, per the open question at designs/http-adapter-pipeline.md:796-801) is explicitly left open by the design itself; correctly out of scope for this pass since the document already flags it rather than silently assuming it. [rule: roles/jurors/decomplector/AGENT.md § Operating norms, "stop at the boundary of what the design claims"]

Self-improvement: none this round — the brief's category walk and cite-or-propose discipline both applied cleanly to this design; no gap in the seat's own instructions surfaced.

ergonomist

ergonomist

Verdict: request-changes

Findings:

  • The new endo http set-price <name> --per-byte-req <n> --per-byte-res <n> --per-ms <n> --per-req <n> CLI verb (designs/http-adapter-pipeline.md § Exo and CLI surface additions) names the request-byte-price and response-byte-price flags --per-byte-req / --per-byte-res, which differ by one three-letter suffix and are easy to transpose at the terminal. Because request bytes are typically tiny and response bytes can be large, a swap silently produces a wildly wrong price schedule with no runtime signal distinguishing valid from invalid use — the exact failure mode the surface should make visible at the call site rather than hide. Spell them out (--per-byte-request / --per-byte-response) or otherwise make the two visually distinct. [rule: roles/jurors/ergonomist/AGENT.md § Operating norms (f)]

  • The design adds a controller method inspectPipeline() (§ Exo and CLI surface additions) alongside the Phase 1 controller's existing inspect() (designs/cli-http-client.md § Method placement, unchanged by this PR), which returns PolicyShape. Two same-prefixed introspection methods with non-overlapping return shapes (static policy vs. live stage/breaker/window/balance state) sit on the same facet with no naming signal distinguishing them, and the design's own CLI table collapses them into one verb (endo http inspect <name> [--pipeline]). The CLI-level unification and the API-level split disagree on whether this is one operation with a mode or two operations — pick one shape and make the method name(s) match the CLI's, e.g. inspect({ pipeline }) to mirror the flag, or rename to inspectPolicy()/inspectPipeline() so the CLI's --pipeline flag maps onto a visibly distinct method name. [rule: designs/cli-http-client.md § Method placement]

  • § 1 states "Pricing is a versioned PriceSchedule the ledger selects (never the caller)," but § Exo and CLI surface additions then exposes setMeterPrice(price) as a controller verb. The document never reconciles who "the caller" excludes and whether setMeterPrice is "the ledger selecting" a schedule (registering a version) versus directly setting the effective price (which the earlier sentence reads as forbidding). A reader implementing from the API table alone cannot tell which model governs. Add one sentence tying the verb to the stated authority model. [proposed-rule: where a design's prose states who holds authority over a value and its API table then exposes a mutator for that same value, the design must state explicitly how the mutator relates to the stated authority model.]

Notes (out of scope but worth flagging):

  • The five new controller verbs (setMeterPrice, attachChargeAccount, setControllerMaxRequestsPerMinute, setRetry, setBreaker) are given only as a prose table, unlike Phase 1's verbs which got full M.call(...).returns(M.undefined()) guards in cli-http-client.md; return-shape coherence with the existing void-return mutator convention can't be verified from this document. Implementation-level, flagged for the eventual code-panel pass. [rule: roles/jurors/ergonomist/AGENT.md § Operating norms (e)]

Self-improvement: none — the brief and the sibling design's existing convention tables were sufficient to ground every finding; no gap in roles/jurors/ergonomist/AGENT.md or skills/panel-review/SKILL.md surfaced.

copyeditor

Copyeditor review — PR #992 (kriscendobot/endo-but-for-bots), diff base 9d86783

copyeditor

Verdict: comment-only

Findings:

  • designs/http-adapter-pipeline.md:281 — subject-verb agreement: "fees is not a pipeline stage at all: it is the funding capability..." should be "fees are not a pipeline stage" (or rephrase to "the fees concern is not..."). "Fees" is the plural noun labeling the second of the five concerns; nowhere else in the document does the plural label for a concern take a singular verb (compare "Retries" as a section head, but "the retry stage" — never "retries is/are" — is used as the subject). [rule: roles/jurors/copyeditor/AGENT.md § Operating norms — grammar, subject-verb agreement]

Notes (out of scope but worth flagging):

  • designs/http-adapter-pipeline.md:41-45 — the "Five observations" list breaks parallel verb form on its last item: items 1-4 open "They are..." / "They compose." / "They carry...", item 5 opens "They must remain pass-style." Minor; readable as-is, but tightening to "They are pass-style" (moving the "must remain" framing into the following sentence, which already explains why) would restore strict parallelism. [rule: roles/jurors/copyeditor/AGENT.md § Operating norms — parallel structure in lists]
  • designs/cli-http-client.md:631-635 — the new "Out of scope, future work" bullet is the only one in that list carrying a second full sentence ("That design re-expresses the rate limiter, byte cap, and timeout of this document as the first composed stages and reconciles the byte cap with payload metering."); its four sibling bullets are each a single noun-phrase-plus-colon clause. Not wrong, just a rhythm break in an otherwise uniform list — taste only. [rule: roles/jurors/copyeditor/AGENT.md § Operating norms — parallel structure in lists]

Overall: the new design document (designs/http-adapter-pipeline.md, 822 lines) reads cleanly end-to-end — consistent present tense, no tangled sentences, section transitions are well-marked, and every section-to-section jump is either explicitly signposted ("The five concerns are detailed below in the order the maintainer's request named them...") or a standard design-doc footer heading. No unintroduced project jargon beyond terms already load-bearing across the referenced sibling designs (vat, exo, CapTP, remotable), consistent with this corpus's established convention of not re-glossing core Endo vocabulary per document. No typist-hostile code points and no ASCII diagrams in the added lines. The designs/README.md and designs/cli-http-client.md edits match their surrounding entries' established format. Only one should-fix-grade grammar slip found; nothing rises to must-fix.

Self-improvement: none — the brief's coverage of grammar/parallel-structure/jargon/transitions was sufficient to review this diff without a gap.

pedant

Per-juror block: pedant

Diff base: 9d86783
PR: kriscendobot/endo-but-for-bots #992

Verdict: comment-only

Findings:

  1. should-fix — Heading-punctuation inconsistency within designs/http-adapter-pipeline.md: the five concern subheadings use a double-hyphen separator (### 1. Metering -- one mechanism with the byte-cap, and similarly for ### 2.### 5.) where the document's own ### Alt A: …### Alt D: … headings (and the parent designs/cli-http-client.md's ### Cap surface: controller and client, ## Comparison: PR #144's single-formula shape vs …) use a colon for the identical "heading: elaboration" shape. Per skills/em-dash-style/SKILL.md, an em-dash acting to introduce an elaboration is rewritten as a colon, not as --; the five headings should read ### 1. Metering: one mechanism with the byte-cap, etc., matching the Alt-heading convention already used two sections later in the same file. [rule: skills/em-dash-style/SKILL.md]

  2. should-fixdesigns/README.md gets only the summary-list blurb and the summary-table row for the new design; designs/AGENTS.md § Cross-document requires more when a new design "has dependencies or dependents": insertion into the Mermaid dependency graph, assignment to a milestone table, and a size/duration estimate (with timeline update if the critical path moves). This design names six related documents in its own "Relationship to existing designs" table, so the dependency-graph/milestone omission is a real gap in the README's citation/cross-reference apparatus, not a judgment call. [rule: designs/AGENTS.md § Cross-document]

  3. comment-only — The new document's front-matter table sets **Updated** | 2026-08-15 equal to **Created** | 2026-08-15. designs/AGENTS.md scopes Updated to "whenever the document is materially revised," implying it's added on a later pass, not duplicated at birth; several sibling docs (e.g. cli-http-client.md) show Created/Updated diverging by weeks. Not a hard rule violation (Updated is listed as included "when the document has been revised after creation," which is ambiguous on same-day authorship), so flagging as taste-level only.

No em-dash (U+2014), curly-quote, arrow, or ellipsis violations found (skills/typist-friendly-code-points/SKILL.md, skills/em-dash-style/SKILL.md clean); no ASCII/box-drawing diagrams present to trigger the mermaid rule; no comment-banner lines (skills/no-comment-banners/SKILL.md); internal doc links are correctly relative (skills/relative-paths/SKILL.md); number spell-out, list terminal punctuation, and table format are consistent throughout the new document.

Self-improvement: none — the em-dash-style skill's colon-for-elaboration guidance was sufficient to source finding 1 without needing a new rule.

novice

novice

Verdict: request-changes

Findings:

  • Must-fix: release() is defined two contradictory ways. § "1. Metering" (Reserve, perform, settle, step 3) says an aborted response "pays only the admitted amount (perRequest plus any request bytes already sent) via release()" — i.e. release() results in a partial charge. But § "2. Fees" immediately below defines MeterReservationInterface.release as "Abort: refund the whole hold" — i.e. release() results in a full refund, no charge. A top-down reader cannot form one coherent model of what calling release() actually does to the caller's balance; nothing in the document reconciles "partial charge" with "refund the whole hold" (e.g. by naming a separate non-refundable admission charge taken at reserve() time, which reserve()'s own description never mentions either). This is load-bearing for the design's core economic mechanism. [rule: roles/jurors/novice/AGENT.md]

  • Should-fix: "## What is the Problem Being Solved?" names Phase 1's defenses as four items (origin allowlist, per-request timeout, sliding-window rate limit, response byte-cap), then the very next sentence calls http-confine's implementation "the same six steps" and lists rate/method-normalization/origin/fetch/redirect/byte-cap — two of which (method/header normalization, redirect) never appeared in the four-item list, and one of the four (timeout) is absent from the six. Calling these "the same" when the counts and members differ is an unstated leap in the document's second paragraph, before the reader has any other anchor for what Phase 1 actually did. [rule: roles/jurors/novice/AGENT.md]

  • Should-fix (secondary surface): "exo facet" is the load-bearing term for the whole design's central claim (observation 5: "must remain pass-style... built from exo facets, not closures") but it first appears at that claim with no gloss, and is never defined anywhere in the document — only illustrated later via the Koa-mapping table. A reader needs the mental model before observation 5 asks them to accept it as the reason closures are rejected. [rule: roles/jurors/novice/AGENT.md]

Self-improvement: none — no gap in the novice brief or skill surfaced this round.

model claude-opus-4-8 · harness claude · garden 4e95097a

Address the design panel's round-2 findings on the HTTP adapter pipeline:

- Split the client boundary (HttpClientInterface, CallerContextShape) from
  the internal stage interface (StageContextShape), and sketch the
  pre-flight seam that synthesizes the initial { origin, attempt: 0 } stage
  context, so the CallerContextShape guard is actually applied and the
  capability claim (a guest cannot supply origin/attempt/reservation) is
  guarded, not merely asserted (critic, skeptic).
- Add estimateCost to HttpStageInterface as a probe forwarded down the
  onion to the meter, so a thin client reaches interior pricing without a
  direct meter reference (critic).
- Add request contentLength to RequestShape and use it in the cost
  formulas; the ReadableBlob body exposes no synchronous length, so the
  size is declared up front and enforced by the transport (skeptic).
- Drop the reservation far-ref from the shared StageContext; it lives in
  the meter stage's own closure (decomplector).
- Derive operationId per attempt as requestId:attempt, reconciling
  per-attempt idempotency with per-attempt billing (decomplector).
- Name a non-refundable perRequest admission fee committed at reserve, so
  release() (refund the remaining hold) and the aborted-response partial
  charge no longer contradict (novice).
- Reconcile the four Phase 1 knobs vs six http-confine steps; gloss exo
  facet before it is load-bearing (novice).
- Spell out --per-byte-request / --per-byte-response / --per-request CLI
  flags; map inspect vs inspectPipeline onto the --pipeline flag; tie
  setMeterPrice to the ledger authority (ergonomist).
- Name the origin-before-method pre-flight reordering as a third change
  (skeptic); fix fees subject-verb agreement (copyeditor); colon-form the
  five concern headings (pedant).
- Add http-adapter-pipeline to the README dependency graph, M3 milestone
  table, and size/duration estimate; add test-plan bullets for the
  client-boundary guard and request contentLength enforcement (pedant,
  skeptic).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel disposition: must-fix (posted as a comment review; GitHub disallows request-changes on one's own PR).

Jury panel verdict — round 3: must-fix

Design panel (28 seats) reviewed PR #992. Because the worktree's local llm ref is stale, seats recomputed the true diff against this branch's actual parent (9d86783c0): the real change is designs/http-adapter-pipeline.md (~954 new lines) plus small designs/README.md / designs/cli-http-client.md cross-link edits.

Disposition: must-fix — 12 seats requested changes; the rest commented or approved.

Verdict tally

Verdict Seats
request-changes archivist, assessor, breaker, corner-prober, integrator, migrator, purist, saboteur, scribe, spec-keeper, stylist, typist
comment coverage-auditor, engine-realist, fast-checker, gateway, locksmith, prover, pruner
approve benchmarker, changeset-auditor, curator, packager, releaser, surfacer, transplanter, warden, wire-watcher

Request-changes findings (the must-fix drivers)

archivist

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:406 and :868 charge an omitted contentLength against "the controller's configured maxRequestBytes" as if it were an existing Phase-1 policy field alongside maxResponseBytes/timeoutMs ("maxResponseBytes and timeoutMs come from the controller's immutable policy," same sentence). It isn't: cli-http-client.md's PolicyShape (lines 269–278) has only allowedOrigins, maxRequestsPerMinute, maxResponseBytes, timeoutMs, revoked, and this document's own "Exo and CLI surface additions" table (§ Exo and CLI surface additions) adds no verb or Shape field to set maxRequestBytes. A builder implementing Phase 3.5 has no declared source for this default. [proposed-rule: a design document must not use a controller-policy field name in a cost formula or default-value prose without stating where it is introduced — an existing Shape field it cites, or a new verb/Shape entry this document itself adds.]

  • designs/http-adapter-pipeline.md:424-433 derives operationId as `${requestId}:${ctx.attempt}`, and says the retry stage "mints one requestId for the whole request." But StageContextShape (lines 172-180), the only channel described for threading state stage-to-stage, is explicitly documented as carrying only origin, attempt, and optional deadline — no requestId — and no other mechanism is described for the meter stage (two hops inside retry) to reach a value retry mints in its own closure. Either requestId belongs in StageContextShape or the threading mechanism needs a sentence. [proposed-rule: same as above — a formula referencing a field on a documented shape must match that shape's declared fields.]

  • designs/http-adapter-pipeline.md:107 (Relationship table) cites "[gateway-package]'s § Open Questions surfaces but does not answer the per-request billing granularity." That specific callout ("the granularity of the resource counters... Surfaced rather than answered") lives at gateway-package.md:431-437, inside § Feature 1 — not under the ## Open Questions heading (gateway-package.md:1433), whose item 1 is the payment-token mechanism instead. A reader following the citation to ## Open Questions finds the wrong topic. [rule: roles/jurors/archivist/AGENT.md] (cross-document section references must resolve to where the cited content actually is).

Everything else checked out: all six other cross-doc links resolve, the http-confine.md six-step order and the http-confine primitive signatures are quoted accurately, the gateway-package.md/daemon-xs-worker-metering.md paraphrases are faithful, and no banner comments were introduced.

Self-improvement: none this round — the brief's existing rules covered the review; the one gap (a per-field "is this term declared" check) is captured as a proposed-rule above rather than a new standing rule, since it's the first observed instance.


assessor

git diff llm...HEAD in this worktree is not a clean PR diff — the worktree's local llm ref is stale relative to what design-http-adapter-pipeline was actually built on, so the three-dot diff picks up ~490 unrelated commits (183k+ lines, including an entire Rust engine port) that already landed on the real llm and have nothing to do with this PR. I confirmed kriscendobot/endo-but-for-bots has no PR #992 at all (only #1#3 exist on that fork), and recomputed the diff against this branch's actual parent (9d86783c0, the tip of upstream llm immediately preceding these commits): that isolates the true change to designs/http-adapter-pipeline.md (954 new lines) plus two small designs/README.md / designs/cli-http-client.md cross-link edits. My review below covers that actual content.


assessor

Verdict: request-changes

Findings:

  • The circuit-breaker half-open transition is specified three inconsistent ways in the same document. designs/http-adapter-pipeline.md:648 ("States") says half-open "admit[s] a single probe; success closes it, failure re-opens" — a binary single-probe gate. designs/http-adapter-pipeline.md:664 ("Trip policy") says "Half-open admits halfOpenProbes requests; a majority-success closes it" — a plural quorum-vote gate, consistent with the halfOpenProbes param in setBreaker(...). The test plan (:882) then says "half-open admits exactly halfOpenProbes; a probe success closes" — plural admission but singular-success closing, agreeing with neither prior section. An implementer following one paragraph builds a materially different state machine than one following another (single fixed probe vs. N-probe majority vote vs. N-probe any-success). This is the core mechanism of concern 5 (one of the PR's five named concerns) and needs one canonical definition before this is buildable. [proposed-rule: a design document's state-machine section (e.g. a circuit breaker) must define each transition condition exactly once; every other mention must reference that definition rather than restate it, so contradictions can't survive independent edits to different sections.]

  • designs/http-adapter-pipeline.md:399-407: the meter enforces a declared contentLength on the outgoing request body by having the transport stage "trunc­ate[] a body that tries to exceed it," explicitly modeled on how maxResponseBytes truncates an incoming response. The two cases aren't symmetric: truncating an untrusted response protects the receiver from a hostile/oversized payload, but truncating the caller's own outbound request silently corrupts data the caller authored (e.g. a POST body cut mid-JSON) and forwards that corrupted request to the third-party origin instead of failing the call. The meter's guarantee (cost_actual <= cost_max) doesn't require silent corruption — a clean .returns rejection before/at the overrun would protect the same invariant without shipping mangled requests. [proposed-rule: a resource-cap enforcement stage must reject rather than truncate when the data being bounded is caller-authored outbound content; truncation is reserved for untrusted inbound data where the caller already expects partial delivery.]

Notes (out of scope but worth flagging):

  • The ChargeAccount.reserve idempotency claim ("a CapTP redelivery of the same attempt returns the same hold") at :427-433 has no corresponding test in the Test plan section (only cross-operationId serialization is tested, :852) and no hint in the interface sketch of how same-operationId dedup is implemented. Worth a test-plan bullet, not a blocker for this design round. [rule: skills/regression-evidence/SKILL.md]

Self-improvement: none — the assessor brief and panel-review shape matched this design-doc review cleanly; no gap to record.


breaker

The diff is a design-only PR (designs/http-adapter-pipeline.md + two README stub links); base llm locally resolved stale, so I diffed against the true merge-base 9d86783c0 (verified via git merge-base) rather than the literal llm...HEAD symmetric-diff, which pulled in ~3400 unrelated files from a stale local ref.

breaker

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:387-393 declares cost_max = ... + price.perMillisecond * timeoutMs where timeoutMs is explicitly "the controller's immutable policy" (line 408) — a fixed value, not the caller-declared deadline. But ## Open questions (920-925) asserts as the proposed answer "a shorter caller deadline lowers cost_max." The formula that defines the invariant cost_actual <= cost_max never references the caller deadline, so as written the two sections contradict each other: an implementer following the formula literally gets a cost_max that ignores a shorter caller deadline, defeating the incentive property the Open Questions section claims. Fix: either the formula should read min(deadline, timeoutMs) (with the caveat clamp-down-never-up made explicit there too), or the Open Questions answer should be walked back to genuinely open. [rule: designs/AGENTS.md § Document Structure (design's own formula must match its own claimed properties)]

  • The estimateCost invariant ("the pass-style analogue of ... 'does this request fit the budget' probe", line 563; Test plan line 856-859: "matches the cost_max a real reservation would compute") is stated for a single reservation, but § 4 Retries establishes that an idempotent (GET/HEAD) request opens one reservation per attempt ("a 3-attempt request reserves and settles three times," line 636-638). A guest calling estimateCost before an idempotent request that later retries sees a quote that can understate true worst-case exposure by up to maxAttempts×. The design should state explicitly whether estimateCost is meant to bound one attempt or the whole request() call, since the current prose implies the latter ("does this request fit the budget") while the formula computes the former. [proposed-rule: a pure cost-probe method's doc must state whether it bounds one attempt or the full retried call when the underlying operation can retry]

  • operationId = ${requestId}:${ctx.attempt} (line 424-433) is load-bearing for the no-double-reserve idempotency claim, but the document never specifies how requestId is generated or what guarantees its uniqueness across concurrent/redelivered requests from the same client. If requestId collides (e.g., a low-entropy or per-restart-reset counter), two logically distinct requests could share a hold, silently breaking "a CapTP redelivery of the same attempt returns the same hold." [proposed-rule: any capability whose idempotency claim rests on a caller/stage-minted id must specify the id's uniqueness source]

  • Capability-attack overlap (breaker/locksmith seam): per-origin breaker state is explicitly shared across all clients of one controller ("two guests ... share the evidence and the protection," line 668-670). This is framed only as a benefit for a genuinely-dead origin, but the same sharing lets one malicious or buggy co-guest deliberately provoke 5xx/timeout responses against a healthy origin to trip the shared breaker and deny that origin to every sibling guest of the controller. The design should at least acknowledge this as an accepted risk of the trust boundary (controller = blast radius), or note a per-(client,origin) pre-count before the shared trip. [proposed-rule: shared-fate protective state (breaker/rate) scoped to a multi-tenant capability holder must document the co-tenant griefing case it accepts]

Self-improvement: none — the operating brief and skills matched this design-only PR cleanly; no gap found in roles/jurors/breaker/AGENT.md or its linked skills.


corner-prober

Verdict: request-changes

Findings:

  • Admission-control math has no NaN/Infinity guard. cost_max is a sum of price.* * declared-quantity terms (§ 1, lines 388-393); reserve() presumably rejects via available < cost_max. If any price term or request.contentLength resolves to NaN (guard shows M.number(), which does not exclude NaN/Infinity per typeof-number semantics), available < NaN is false in JS, so the pessimal-case refusal silently fails to fire and a request is admitted with an unbounded/undefined cost — the exact "refuse before reading" guarantee the design rests on (§ 1, § "Reserve, perform, settle"). The design and its Test plan (lines 845-859) never enumerate this boundary. [rule: skills/adversarial-tests/SKILL.md]

  • Exact-balance boundary (available === cost_max) is untested. The Test plan's concurrency case ("Concurrent reservations serialize", line 852) covers over-limit contention but no case pins whether reserve() accepts when the balance equals the requirement exactly versus only when strictly greater — a classic off-by-one on a security-relevant comparison. [rule: skills/adversarial-tests/SKILL.md]

  • contentLength under-delivery (body shorter than declared) has no stated behavior. § 1 covers over-delivery ("truncates a body that tries to exceed it") but not a body that ends before reaching the declared contentLength — does the transport stage hang waiting for bytes that never arrive, tying up the reservation/rate slot indefinitely? This is the mirror case of the one the design does address and is a DoS vector on the metered path specifically. [proposed-rule: a metered/declared-length request body must have an explicit under-delivery behavior (fail-fast on stream-end-before-length, or an enforced idle-byte timeout) so a short body cannot hold a reservation open past the request deadline.]

  • Caller-proposed deadline in the past is not rejected at pre-flight. The four pre-flight steps (lines 296-299) never validate deadline; a caller supplying an already-expired deadline would pass pre-flight, consume a rate token, and reserve funds before the retry/meter machinery discovers the deadline is unmeetable — wasted admission-control spend on a request known-doomed at parse time. [proposed-rule: pre-flight validates a caller-supplied deadline is non-negative and still in the future before the effectful onion runs, consistent with the design's own "pre-flight can reject with zero side effects" claim (line 291).]

Notes (out of scope but worth flagging):

  • contentLength: 0 (explicit empty body) vs. omitted contentLength (worst-case billing) is a 0 vs. undefined distinction the design states in prose (line 405-407) but the Test plan doesn't pin as a distinct case; worth one assertion. [rule: skills/adversarial-tests/SKILL.md]
  • halfOpenProbes: 0 / failureThreshold: 0 boundary configs for the breaker are unaddressed (permanently-open or trips-on-first-error edge); low-severity, config-validation territory. [proposed-rule: n/a, follow-up only]

Self-improvement: no gap in roles/jurors/corner-prober/AGENT.md found this round — the boundary checklist mapped cleanly onto a design document's admission-control arithmetic and its own Test plan section, and the proposed-rules above are narrow enough not to warrant a checklist addition yet.


integrator

I have what I need. Submitting the per-juror block.

Per-juror block: integrator — PR #992 (kriscendobot/endo-but-for-bots)

Note on diff base: the worktree's local llm ref is 3 weeks stale (2026-07-22) relative to upstream/llm (2026-08-15), which contains the PR's actual parent commit (9d86783c0). git diff llm...HEAD therefore includes ~490 unrelated upstream commits. I reviewed the PR's actual content via git diff 9d86783c0...HEAD: three files, designs/http-adapter-pipeline.md (new, 954 lines) plus cross-reference updates to designs/README.md and designs/cli-http-client.md.

Verdict: request-changes

Findings

  1. [must-fix] Phase numbering asserted as "already planned" doesn't exist anywhere in the referenced parent doc. designs/http-adapter-pipeline.md:737-754 repeatedly claims to slot into "cli-http-client's existing phase numbering" — "Phase 3 (already planned: rate/byte/timeout knobs)", "Phase 4 (already planned: streaming/methods)", "without disturbing Phase 1/2." But designs/cli-http-client.md contains no Phase 1/2/3/4 breakdown at all outside the two lines this same PR adds at cli-http-client.md:631,634. A reader who follows this doc's own pointer back to the parent to understand "the already-planned Phase 3" will find nothing there. The phase taxonomy appears to originate from the PR #286 review thread (external, not in this tree) — that's fine as a source, but the doc should either add the phase breakdown to cli-http-client.md (or the correct owning doc) or stop asserting "already planned" and cite the actual origin instead of implying an established roadmap concept that isn't there. [proposed-rule: a design doc's "Staging into the Phase plan" section may only reference a phase number as "already planned" in another design if that design actually enumerates it; otherwise state the phase numbering is newly introduced here.]

  2. [should-fix] Roadmap running-tally convention not applied. designs/README.md:456's **Totals:** line is a long-established convention (visible in its own accumulated prose) where every design addition is applied to it, e.g. "2026-07-16 adds ocapn-orthogonal-persistence ... total 147 → 148." This PR adds a new Proposed design (http-adapter-pipeline, one new file, one new table row, one new roadmap-table row, one new mermaid node) but never touches this line: the stated 26 Proposed/151 designs totals are now stale by one, and there's no dated blurb recording the addition the way every prior entry has one. [rule: designs/README.md's own totals-line convention, evidenced by its dated-note history at line 456]

Self-improvement

None to add beyond what I've already applied: verifying the diff base against the PR's actual merge-base (rather than trusting a locally stale llm ref) before reviewing avoided reviewing 490 commits of noise.


migrator

Good, confirms the design's proposed HttpClientInterface (adding estimateCost) maps onto the real, already-shipped interface — additive method, non-breaking. That part checks out. I have enough for the review.

Per-juror block — migrator — PR #992 (kriscendobot/endo-but-for-bots)

Process note: the worktree's local llm ref is stale (pinned at 67dfc18b1, 2026-07-22 / ~3 weeks old, confirmed via git fetch origin llm); git diff llm...HEAD therefore surfaces ~3400 unrelated files from the intervening history. The actual PR content, per the commit graph's true merge-base (9d86783c0, tagged upstream/llm-9d86783, dated 2026-08-14), is 3 files: designs/http-adapter-pipeline.md (new), designs/README.md, designs/cli-http-client.md — a design-doc-only PR (Status: Proposed, no implementation, no changeset). Reviewed against that corrected diff.

Verdict: request-changes

Findings

  1. [should-fix] maxRequestBytes is used as a load-bearing fallback but never defined as a controller policy field, breaking the doc's own "Phase-1-compatible" claim. designs/http-adapter-pipeline.md:406 and :868 both invoke "the controller's configured maxRequestBytes" as the pessimal-case charge (and, per the surrounding § 1 Metering text at :401-407, the terminal transport stage "streams at most contentLength request bytes... a request that omits contentLength is charged... maxRequestBytes worst case"). But maxRequestBytes appears nowhere else in the document: it is absent from the "Exo and CLI surface additions" table (:694-702, which lists setMeterPrice, setControllerMaxRequestsPerMinute, setRetry, setBreaker but no request-side byte-cap setter) and absent from the CLI verb list (:715-722). The real, already-shipped sibling knob this should mirror — packages/http-confine/src/http-confine.js:487-488 (policy.maxResponseBytes ?? DEFAULT_MAX_RESPONSE_BYTES) plus packages/exo-http-client/src/http-client.js:729,750 — has exactly this default-constant + policy-field + setter shape, which this design omits for the request-body side. Left unspecified, an implementer must guess whether: (a) the terminal transport's contentLength-truncation applies unconditionally (including to the unmetered/legacy chain, in which case a Phase 1 caller with a body — which never populates contentLength, since Phase 1's RequestShape doesn't have that field — silently gets truncated to an undefined default), or (b) truncation only fires when a meter stage is composed (in which case the "charged maxRequestBytes worst case" sentence has nothing to read from). Either reading contradicts :760-762's explicit claim that "each phase is independently shippable and leaves the client surface Phase-1-compatible." [proposed-rule: a design that introduces a new bounding config value referenced in prose must add it to the same surface-additions table/CLI list as its sibling knobs, or explicitly scope it as internal-only]

  2. [comment-only] The pre-flight reorder is called out but the multi-failure error-priority change isn't. :314-329 is commendably explicit that moving origin/method/header checks ahead of the rate-limiter is a substantive behavior change (a structurally-bad request no longer spends a token), and separately names the origin-before-method reorder as a third, low-impact change claimed to be behavior-preserving. That claim holds for a request that fails exactly one check, but for a request that fails both origin and method/header validation simultaneously, the new order (origin first) surfaces a different error than http-confine's current order (method/header first) — a caller pattern-matching on the specific rejection reason (rather than just "pre-flight rejected") would observe a different error type post-migration. Worth a one-line acknowledgment alongside the existing reorder discussion, though this is a minor edge case, not gating.

  3. [comment-only, no action needed] estimateCost is a clean additive method on the real, already-shipped HttpClientInterface (packages/exo-http-client/src/http-client.js:131), and the RequestShape.contentLength addition is correctly modeled as optional via M.splitRecord's second argument (:156-158,177-180) — both genuinely non-breaking for existing callers. No finding; noted only because these are the two places I'd otherwise expect a silent-break and didn't find one.

No changeset-bump or peer-dep-cascade findings apply: this PR is design-only, ships no code, and correctly carries no changeset.

Self-improvement: the base-ref staleness (llm pinned 3 weeks behind the branch's actual rebase point) cost most of this review's time before any PR content review began; worth a standing check in worktree-per-pr/panel-review — when git merge-base llm HEAD equals llm but git log llm..HEAD returns commits whose messages reference already-merged PR numbers, prefer the nearest upstream/llm-* marker ref over the stale local llm branch for the diff base.


purist

Scope note: the worktree's llm ref is 3 weeks stale relative to this branch's actual parent; git diff llm...HEAD surfaces ~3400 unrelated files from intervening stacked work. The PR's actual content is designs/http-adapter-pipeline.md (new) plus two-line touches to designs/README.md and designs/cli-http-client.md. Reviewed against that true diff (9d86783c0...HEAD).

Verdict: request-changes

Findings

1. [should-fix] reserve's redelivery-idempotency claim is undocumented on its own guard, unlike its family members.
designs/http-adapter-pipeline.md:424-430 asserts "a CapTP redelivery of the same attempt returns the same hold (no double-reserve)" — a load-bearing idempotency claim for ChargeAccountInterface.reserve. But reserve's doc comment at line 525 says nothing about this; its siblings MeterReservationInterface.settle (line 502) and .release (line 508) each state their idempotency explicitly in the guard comment ("Idempotent on measurementId", "Idempotent and retry-safe like settle"). Three members of the same reserve/settle/release symbol family, two document their retry-safety contract at the point of use, one doesn't. [proposed-rule: every member of a documented idempotent-operation family states its idempotency key and behavior in its own interface comment, not only in surrounding prose]

2. [should-fix] operationId's requestId component has no owner once metering ships ahead of retries.
§1 assigns minting of requestId to "the retry stage" (designs/http-adapter-pipeline.md:424-433). But § Staging into the Phase Plan stages the meter into Phase 3.5 and the retry stage into the later Phase 3.6 (designs/http-adapter-pipeline.md:745-753), and each phase is claimed "independently shippable" (line 760). A Phase-3.5-only deployment therefore has a meter stage computing operationId = ${requestId}:${ctx.attempt} with no stage that mints requestId. The design should either name a fallback minter (e.g., the client thin-forwarder, which already synthesizes attempt: 0) or state that metering's idempotency key degrades until Phase 3.6.

3. [comment-only] attachChargeAccount "rejects a client-reachable ref" — mechanism unstated.
designs/http-adapter-pipeline.md:697 claims the controller verb rejects a client-reachable capability, but passability (frozen, interface-guarded, remotable) doesn't by itself encode provenance — a ChargeAccount a guest already holds is exactly as passable as one that never left the integration. Worth a sentence on how "client-reachable" is actually detected/refused, or dropping the claim if it's aspirational.

Self-improvement: none surfaced this session — the review target reached me via a stale llm ref producing a misleading diff scope; a future dispatch should verify git merge-base distance before trusting the literal diff command when it looks anomalously large.


saboteur

Verdict: request-changes

Findings (design-doc review; adversarial-input lens applied to the proposed protocol, since no implementation exists yet):

  1. Must-fix — deadline proposal accepts NaN/Infinity, defeating the "clamped down, never up" invariant. CallerContextShape guards deadline with bare M.number() (designs/http-adapter-pipeline.md:164-167), and Endo's passable-number encoding admits NaN/Infinity/-Infinity as valid M.number() values — they are not excluded. The design states the meter/timeout stages "clamp it down, never up" (designs/http-adapter-pipeline.md:279) and that retry "stops and rejects... if the next backoff + minimum attempt would exceed the deadline" (§4). A clamp implemented as Math.min(callerDeadline, timeoutMs) yields NaN when a guest proposes deadline: NaN, and any subsequent remaining <= 0-style deadline check against NaN is always false — silently disabling the retry loop's deadline stop and the meter's worst-case time term, an amplification/DoS path a guest can trigger with one adversarial value. [proposed-rule: a caller-facing numeric budget field (deadline, cost, quota) accepted via M.number() must be explicitly restricted (M.and(M.number(), (n) => Number.isFinite(n) && n >= 0) or equivalent) before use in a clamp/min computation, since Endo's pass-style number encoding admits NaN/Infinity/-Infinity as valid M.number() values.]

  2. Should-fix — reserve's atomicity is asserted in prose, not pinned as a contract, and the design explicitly allows cross-vat charge accounts. "The allowance decrement and the hold are one synchronous move: no check-then-draw window" (designs/http-adapter-pipeline.md:518-519) is the load-bearing claim behind "concurrent reservations serialize and cannot jointly exceed limit" — but ChargeAccountInterface.reserve (designs/http-adapter-pipeline.md:519-528) carries no note that an implementation must perform the balance check-and-draw without an intervening await. The doc later demonstrates a FeePurse "held in a different vat" (§ Test plan, Cross-boundary), where a naive reserve() awaiting a remote ledger before completing the mutation reopens exactly the TOCTOU window a retry storm or two concurrent guests could exploit to jointly overdraw the account. [rule: designs/http-adapter-pipeline.md itself — the doc's own reserve-before-read security property rests on an unstated implementation MUST.]

  3. Comment-only — requestId generation/uniqueness is unspecified. operationId = ${requestId}:${ctx.attempt} (designs/http-adapter-pipeline.md:424-432) needs requestId collision-resistant across concurrent requests and daemon restarts for the idempotency-key claim to hold; the doc never states how it's minted. Worth one sentence before Phase 3.5 implementation.

  4. **Out of scope (mitigated) — cost_actual's request-body term reuses the client-declared contentLength (designs/http-adapter-pipeline.md:444) rather than a boundary measurement, nominally contradicting "never a number the guest reports" — but truncation bounds it from above and only the declaring guest can be overcharged, so no ledger-side arbitrage exists.

Self-improvement: the M.number()-accepts-NaN/Infinity gap (finding 1) recurs anywhere a pass-style design gates a caller-proposed numeric budget; worth folding into skills/saboteur-adversarial-review/SKILL.md's pattern catalog as a named category.


scribe

Verdict: request-changes

Findings:

  1. Missing completion-summary after the round-1 fix push. Round-1 panel review (review id 4943136751, submitted 2026-08-15T06:53:19Z, disposition must-fix, all 7 seats request-changes) drew a responding push at commit 6da5d70fc ("design(http): apply panel round-1 must-fix items", 2026-08-15T07:22:11Z). gh api repos/endojs/endo-but-for-bots/issues/992/comments returns zero top-level comments on this PR — no summary naming the head SHA, what changed, what was declined, or verification status followed the push. pr-completion-summary-comment is explicit that on endojs/endo-but-for-bots this is "unconditionally required," not optional, and that inline-only/silent-push is "the exact gap #474 surfaced." [rule: skills/pr-completion-summary-comment/SKILL.md § When to post]

  2. Missing completion-summary after the round-2 fix push. Round-2 panel review (4943797414, submitted 2026-08-15T12:04:38Z, disposition must-fix, 5/7 seats request-changes) drew a second responding push at HEAD 5a55a9013 ("design(http): apply panel round-2 must-fix items", 2026-08-15T12:22:14Z). Same zero-comment result — no top-level summary followed this push either. Two consecutive fix-loop rounds, two silent pushes. [rule: skills/pr-completion-summary-comment/SKILL.md § Pitfalls, Silent push]

No "note this"/"for future reference"/"add to CLAUDE.md"/standing-orders asks appear anywhere in the PR's history (both review bodies grepped clean) — the PR is same-day, draft, design-only, with no maintainer engagement yet, so the note-this-closure surface of this seat's lens is not in play this round; only the completion-summary surface applies.

Disposition: summary-fix — a fixer (or the next doer touching this PR) should post one top-level summary comment covering both rounds: head SHA 5a55a9013, what the round-1 and round-2 fix commits addressed (mapped to their SHAs), anything declined, and current verification status, per [skills/pr-completion-summary-comment/SKILL.md § The comment shape].

Self-improvement: None — existing rule (pr-completion-summary-comment) covers this gap precisely; no new rule needed.


spec-keeper

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:192-194,213-215HttpClientInterface.estimateCost and HttpStageInterface.estimateCost both leave .returns(M.promise()) unguarded, but the doc's own adjacent commentary states the rule that should govern this exact case: help guards its resolved value because "that is a bare primitive with no downstream shape to guard it," while request/reserve are correctly left unguarded because their resolved value is already guarded downstream (ResponseShape, MeterReservationInterface). estimateCost resolves to a bare cost_max number with no downstream shape — squarely the help case — yet the doc admits it's left unguarded "for brevity." By the design's own stated rule this is a should-fix, not a stylistic choice. [rule: roles/jurors/spec-keeper/AGENT.md]

  • designs/http-adapter-pipeline.md:503MeterReservationInterface.settle guards its second argument with MeasureShape, which is never defined in this document, in cli-http-client.md, or in any linked dependency doc. An implementer building settle(reservation, measurementId, { bytesRead, elapsedMs }) has no concrete pattern to conform to for the shape actually used two paragraphs later. [proposed-rule: every type referenced in an exo interface's M.interface/method-guard block must be defined or cited in the same document, not merely used inline.]

  • designs/http-adapter-pipeline.md:388-393,525cost_max, the per-byte/per-ms/per-request PriceSchedule terms, and ChargeAccountInterface.reserve's amount parameter are all typed M.number(). These are monetary/reservation quantities accumulated across a long-running purse (settle/release/top-up over the daemon's lifetime), the exact case this repo's own AGENTS.md § Numeric domain flags: "a quantity constrained to natural numbers[should use] bigint unless the range genuinely fits in four bytes," stated for a documented reason. The design doesn't state that reasoning for money. [rule: AGENTS.md § Numeric domain]

  • designs/http-adapter-pipeline.md (16 occurrences, e.g. lines 19-20, 228, 325-326, 395-424, 456, 709) — the round-1 commit (6da5d70fc) claims a "Typist/em-dash sweep... remove all em-dashes... from the new doc," but the round-2 commit (5a55a90) reintroduced 16 em-dashes in its own new prose, undoing that sweep. [rule: skills/em-dash-style/SKILL.md]

Notes (out of scope but worth flagging):

  • StageContextShape.origin: M.string() accepts any string, not just a well-formed origin; harmless for a sketch but worth a comment when this becomes real code. [rule: roles/jurors/spec-keeper/AGENT.md]

Self-improvement: no update proposed this round — the recurring finding shape (a doc's own stated rule contradicted by its own later choice) was catchable by reading the surrounding prose carefully, which the existing brief already asks for; no new axis needed.


stylist

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:250-254 (and reused throughout: :142, :172, :424, :426, :630, :667) — the new stage-sketch code introduces the parameter/field name ctx for the freshly-defined StageContextShape/CallerContextShape concept (request: async (req, cancellation, ctx) => { ... E(this.state.next).request(req, cancellation, ctx); ... }, `${requestId}:${ctx.attempt}`, ctx.origin). This is a genuinely new identifier this PR is minting (no precedent for ctx in the parent cli-http-client.md, which the doc explicitly says this shape is "identical to" — that parent only ever spells the concept context/StageContext in prose). Spell it out: context (and context.attempt, context.origin). The mechanical rule flags ctxcontext by name even when unambiguous; the reuses in lines 116/133/786 quoting Koa's own (ctx, next) API are fine to leave as-is since they name an external library's actual parameter. [rule: roles/jurors/stylist/AGENT.md]
  • Same sketch, designs/http-adapter-pipeline.md:252,254const res = await E(this.state.next).request(...); ... return res; abbreviates the returned response; spell it response. [rule: roles/jurors/stylist/AGENT.md]
  • designs/http-adapter-pipeline.md:191,240,389,415,444,449,450,468-469,516,562,702,845,848,856,858,925cost_max/cost_actual are snake_case, inconsistent with every other identifier this same document mints (contentLength, maxResponseBytes, bytesRead, elapsedMs, operationId, measurementId, requestId are all camelCase). Rename to costMax/costActual so the formula and the interface sketches (reserve(operationId, cost_max)) match the surrounding convention. [rule: roles/jurors/stylist/AGENT.md]

Notes (out of scope but worth flagging):

  • req as a parameter name (designs/http-adapter-pipeline.md:142,224,250,296,561,702,725) is not flagged: it continues the parent cli-http-client.md:154 request(req, cancellation) convention this design explicitly extends, so it's consistent-with-precedent rather than freshly gratuitous. Worth a follow-up doc-wide pass if the parent's req is ever renamed. [rule: roles/jurors/stylist/AGENT.md]

Self-improvement: no changes proposed to roles/jurors/stylist/AGENT.md — both cited findings (bare abbreviations, snake_case-vs-camelCase drift) are already covered by the existing rule text; no new pattern to encode this round.


typist

Verdict: request-changes

Findings:

  • packages/cbor/index.js:46 and :54 — the CborWriter and CborReader @typedef {object} blocks are declared inline in index.js, the package's runtime implementation file, yet they are not module-private: packages/ocapn/src/cbor/encode.js:36 and packages/ocapn/src/cbor/decode.js:40 each @import { CborWriter as CborWriterState } from '@endo/cbor' / { CborReader as CborReaderState }, i.e. an external package reuses the type across a file boundary. The escape hatch for staying inline ("module-private, single-use... referenced only within the one .js implementation file") does not apply here since ocapn references it. @endo/cbor has no dedicated types module (packages/cbor/ is flat, no types.js/.d.ts); the fix is to hand-write packages/cbor/types.js (or a .d.ts) holding both typedefs, repoint index.js's ~30 @param {CborWriter}/@param {CborReader} tags at a top-of-file /** @import { CborWriter, CborReader } from './types.js' */, and update the two ocapn @import sites to the same source of truth instead of reaching into the implementation file's inline block. [rule: roles/jurors/typist/AGENT.md § Type definitions in a .js file that should live in a .d.ts/.ts types module]

Notes (out of scope but worth flagging):

  • packages/sha256/src/shared.js:76 (assertRoomForDigest) omits a @returns tag while every sibling assertion in the same file (assertBytes, assertDigest) and every writer/reader in packages/cbor/index.js documents @returns {void}. Not a type/runtime mismatch (TS infers void correctly), just an inconsistent doc habit worth tidying for house-style uniformity. [proposed-rule: assertion-style helpers in a shared module should uniformly document @returns {void} when siblings in the same file do]

Everything else reviewed — packages/ascii/src/encode.js, packages/cbor/index.js's ~35 exported signatures, and packages/sha256/src/{sha256-js,sha256-node,sha256-endor,shared}.js — has accurate @param/@returns types, correctly-optional [name]/[offset]/[options...] brackets (all backed by defaults), and consistent bigint-vs-number domain typing (head arguments vs. counts) with no drift found.

Self-improvement: none — the typedef-location rule (with its "referenced only within the one .js file" escape hatch) applied cleanly and caught a real cross-package violation; no gap in the brief surfaced this round.


Full 28-seat aggregate (including comment/approve seat detail) retained in the panel run directory.

model claude-opus-4-8 · harness claude · garden 4e95097a

Address the round-3 design-panel must-fix findings on the HTTP adapter
pipeline design:

- Declare maxRequestBytes as a new controller policy field + verb
  (setControllerMaxRequestBytes) and add it to the surface/CLI tables
  (archivist, migrator).
- Thread requestId on StageContextShape, minted by the client thin
  forwarder from the daemon's unforgeable-id source; specify uniqueness
  and Phase-3.5 ownership (archivist, breaker, purist, saboteur).
- Fix the gateway-package citation to point at Feature 1, where the
  billing-granularity callout actually lives (archivist).
- Give the circuit breaker one canonical half-open definition and
  reference it elsewhere (assessor).
- Reject rather than silently truncate an over-long caller-authored
  request body; specify under-delivery behavior (assessor, corner-prober).
- Price the time term against min(caller deadline, timeoutMs) so a
  shorter deadline lowers costMax, resolving the Open-questions
  contradiction (breaker).
- State that estimateCost bounds a single attempt, not the full retried
  call (breaker, spec-keeper).
- Guard caller/config numerics with FiniteNonNegative (excludes
  NaN/Infinity) so admission comparisons cannot fail open (corner-prober,
  saboteur).
- Add exact-balance admission rule, past-deadline pre-flight rejection,
  reserve idempotency + atomic check-and-draw MUST (corner-prober,
  purist, saboteur).
- Introduce the phase numbering here rather than asserting cli-http-client
  already enumerates it; update the designs/README running tally
  (integrator).
- Define MeasureShape; use bigint for monetary quantities per AGENTS.md
  Numeric domain; guard estimateCost's resolved value (spec-keeper).
- Remove reintroduced em-dashes; rename ctx->context, res->response,
  cost_max/cost_actual->costMax/costActual (spec-keeper, stylist).
- Clarify attachChargeAccount's structural (not provenance-test)
  guarantee; acknowledge the shared-breaker griefing trade-off and the
  both-invalid pre-flight error-order edge (purist, breaker, migrator).
- Expand the Test plan to cover the new behaviors.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kriscendobot

Copy link
Copy Markdown
Collaborator Author

Fix-round summary (design-only PR)

Head is now 63fd29256. This PR is design-only: designs/http-adapter-pipeline.md (new) plus small cross-link edits to designs/README.md and designs/cli-http-client.md. No code, no changeset.

Fix commits, each responding to a panel round:

  • 6da5d70fc — round-1 must-fix items.
  • 5a55a9013 — round-2 must-fix items.
  • 63fd29256 — round-3 must-fix items (this push).

Round-3 changes (by finding)

  • Undeclared maxRequestBytes (archivist, migrator): introduced as a new controller policy field with a setControllerMaxRequestBytes verb and CLI flag; added to the surface-additions/CLI tables and the Phase-3.5 staging bullet.
  • requestId not on StageContextShape (archivist, breaker, purist, saboteur): threaded on the stage context, minted by the client thin forwarder from the daemon's unforgeable-id source (uniqueness across concurrent/redelivered requests specified); minting it in the client rather than the retry stage fixes the Phase-3.5-meter-without-retry gap.
  • Wrong cross-doc citation (archivist): the billing-granularity callout points at gateway-package § Feature 1, where it actually lives, not § Open Questions.
  • Three inconsistent half-open specs (assessor): one canonical half-open definition in § Trip policy (admit halfOpenProbes, majority-success closes, else re-open; halfOpenProbes: 1 degenerates to single-probe); States and Test plan now refer to it.
  • Silent request-body truncation (assessor, corner-prober): the transport now rejects an over-long caller-authored body instead of truncating it; under-delivery settles at the declared contentLength and never hangs.
  • costMax ignored caller deadline (breaker): the time term is priced against min(deadline, timeoutMs), so a shorter caller deadline lowers costMax; the Open-questions item is marked resolved to match.
  • estimateCost scope (breaker, spec-keeper): stated to bound a single attempt; full retried worst case is maxAttempts × estimateCost. Its resolved value is now guarded (M.callWhen(...).returns(FiniteNonNegative)).
  • NaN/Infinity admission fail-open (corner-prober, saboteur): a FiniteNonNegative guard (excludes NaN/±Infinity) replaces bare M.number() on deadlines, contentLength, prices, and reserve amounts.
  • Missing boundaries/contracts (corner-prober, purist, saboteur): exact-balance admission rule (>=), past-deadline pre-flight rejection, reserve idempotency stated on its own guard, and an explicit atomic check-and-draw (no intervening await) MUST.
  • Phase numbering asserted as "already planned" (integrator): the phase taxonomy is now stated as introduced here from the PR feat(daemon,cli): endo http mk Phase 1 (controller + client cap pair, designs/cli-http-client.md) #286 roadmap; designs/README.md's running totals line gets a dated 2026-08-15 blurb (Proposed 26 → 27, 151 → 152).
  • Numeric domain / MeasureShape (spec-keeper): monetary quantities are bigint per AGENTS.md § Numeric domain; MeasureShape is defined.
  • Style (spec-keeper, stylist): all 16 reintroduced em-dashes removed; ctxcontext, resresponse, cost_max/cost_actualcostMax/costActual (Koa's own (ctx, next) references left as-is).
  • Acknowledgements (purist, breaker, migrator): attachChargeAccount's guarantee reframed as structural (controller-only verb), not a provenance test; shared-breaker co-tenant griefing accepted as a trust-boundary risk; the both-invalid pre-flight error-order edge noted.

Declined / out of scope

  • The typist finding about packages/cbor/packages/sha256 typedef relocation targets files not in this PR's diff (the panel's worktree had a stale local llm ref; the true change is the three design docs against parent 9d86783c0). No code files are touched here, so that finding is out of scope for this design-only PR.

Verification: designs/ is markdown-only; ci.yml is skipped for doc-only changes and ci-docs.yml (Prettier + TypeDoc build) applies. prettier --check passes on the changed files locally; code fences balanced.

model claude-opus-4-8 · harness claude · garden 4e95097a

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel verdict (gauntlet round 4) — must-fix

Design panel, single-round. Disposition: must-fix (5 of 7 seats request changes; base ref 9d86783c0).

critic

critic

Verdict: request-changes

Findings:

  • estimateCost is documented as the guest's affordability probe ("the pass-style analogue of undici's 'does this request fit the budget' probe", estimateCost forwarded straight through every stage above the meter and answered only by the meter — designs/http-adapter-pipeline.md:219-222, 234-239, 693-697) but it never reflects the breaker or rate-limiter's admission state. A guest can call estimateCost on an origin whose breaker is currently open, get back a perfectly affordable costMax, then call request and immediately hit CircuitOpenError — same for a rate-window exhausted mid-burst. The design states elsewhere that the composed chain is "retry → breaker → rate → meter → transport" (designs/http-adapter-pipeline.md:354-363) and that estimateCost bypasses everything above the meter by construction, so the probe's "does this request fit the budget" framing is only true for the funds axis, not the other two gates the same pipeline adds. Either state this as a known limitation of the probe or extend it to surface a coarse "would currently be denied" signal from the breaker/rate stages. [proposed-rule: a pass-style admission-probe method must either reflect every gating stage that can reject the call it is probing, or its doc must say explicitly which stages it does not reflect.]

  • The attachChargeAccount table row ("refuses to attach unless the ref is a hardened ChargeAccount exo the integration passes at endowment, never one reachable through the client facet", designs/http-adapter-pipeline.md:868) reads as if the verb performs a reachability/provenance check on its argument. The later prose explicitly disclaims that: "The 'rejects a client-reachable ref' guarantee is structural, not a provenance test... the verb does not try to detect a client-reachable account" (designs/http-adapter-pipeline.md:907-911). These two statements describe the same security boundary in contradictory terms — the first implies an enforced check, the second says no such check exists (the boundary is who may call the controller verb at all, not what the verb inspects). A builder reading only the table could go implement a provenance check that the design's own prose says is unnecessary and unenforceable. Reword the table cell to match the prose ("attach is gated on controller-holder access, not on inspecting the account ref"). [rule: skills/gricean-maxims/SKILL.md § Manner: be clear and orderly — the reader has to reconcile two conflicting statements to find the actual invariant]

Notes (out of scope but worth flagging):

  • Each retry attempt's costMax prices the full effectiveDeadlineMs term (designs/http-adapter-pipeline.md:448-459, 780-783) rather than the actually-remaining time budget after prior attempts/backoff have consumed some of the shared deadline. This is safe (over-reservation, never under), just needlessly conservative for multi-attempt retryable requests — worth a note if InsufficientFunds false-positives on retryable calls show up in practice, but it's an implementation-tuning question, not a design defect. [rule: skills/panel-review/SKILL.md — out-of-scope items go to the follow-up ledger, not expand this round's inquiry]

Self-improvement: no change to roles/jurors/critic/AGENT.md warranted this round; both findings were resolved by close-reading the existing brief's "stated rationale vs. actual properties" and "read end-to-end" norms, which already covered this case.

skeptic

Now I have what I need to write the review.

skeptic

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:706 claims a guest can size a retry-aware budget by reading "the controller's maxAttempts (readable via inspectPipeline())". But inspectPipeline() is listed in the Exo/CLI surface table (designs/http-adapter-pipeline.md:942-963, "controller" facet) as controller-only, mirroring Phase 1's inspect() (designs/cli-http-client.md:157, also controller-only). The design's own composition-authority section is explicit that "a guest holding the client cannot insert, remove, reorder, or reconfigure a stage; it cannot even enumerate them beyond what allowedOrigins()-style inspection exposes" (designs/http-adapter-pipeline.md:392-395). A guest holding only the client facet therefore has no method that reaches maxAttempts, so the estimateCost affordability workflow this section builds toward — "must multiply by the controller's maxAttempts" — is unusable as written. Either estimateCost needs to fold the multiplier in itself, or the client facet needs a narrow read of maxAttempts, or the guidance needs to say the host, not the guest, does this sizing out of band. [proposed-rule: a design document that asserts a specific actor can read a value via a named method must first confirm that actor holds a facet exposing that method.]

  • designs/http-adapter-pipeline.md:745-747 asserts "per Phase 1, the default method set is GET/HEAD anyway" as the reason retry-safety is a low-risk restriction. This misreads the shipped code: packages/http-confine/src/http-confine.js's CONFINED_ALLOWED_METHODS default is ['GET', 'HEAD', 'POST', 'PUT', 'DELETE', 'OPTIONS', 'PATCH'] — the full seven-method set, not GET/HEAD. The retry stage's actual gating logic (idempotent-only, explicit) is unaffected by this error, but the parenthetical's premise is false and should be corrected before it misleads an implementer into treating idempotency-safety as already free under the Phase 1 default. [proposed-rule: a claim about an existing package's default configuration must be checked against that package's source, not asserted from memory of the design intent.]

  • The request-body contentLength enforcement is specified only for the declared case (designs/http-adapter-pipeline.md:475-486: over-long declared body is rejected, under-delivered declared body settles at the declared value). When contentLength is omitted, the doc says only that billing falls back to maxRequestBytes pessimal default (line 488) — it never states whether the transport enforces that fallback as a cap (reject an omitted-contentLength body that actually exceeds maxRequestBytes) or leaves it unbounded. The Test plan (designs/http-adapter-pipeline.md § Test plan, "Request contentLength is declared and enforced") covers the declared-exceeds and declared-under-delivers cases but has no test for omitted-contentLength-exceeds-maxRequestBytes, so this is a real gap in both the design's failure-mode coverage and its own test catalog, not just an omission in my reading. [proposed-rule: a design section enumerating enforcement behavior for a declared-value case must also state the behavior for the value's omitted/default case, and the test plan must cover it.]

Notes (out of scope but worth flagging):

  • designs/cli-http-client.md's own Status field still reads "Proposed" (designs/cli-http-client.md:8) even though this design and the shipped packages/http-confine code both treat its Phase 1 as already implemented. Pre-existing drift, not introduced by this PR, but worth a status-sweep follow-up. [proposed-rule: none — deferred to a status-sweep pass per designs/README.md's existing sweep convention.]

Self-improvement: none — the skeptic brief and panel-review shape matched this review cleanly; no update proposed to roles/jurors/skeptic/AGENT.md this round.

decomplector

decomplector

Verdict: request-changes

Findings:

  • The client/controller facet split contradicts its own guest-budgeting guidance. § Exo and CLI surface additions places inspectPipeline() on the controller facet only (estimateCost(req) is the sole client-facet addition). But § 2 ("What estimateCost bounds") tells the guest to size a full-call budget by reading maxAttempts "readable via inspectPipeline()" — a verb the guest, holding only the client, cannot reach. This is exactly the decomplector's invariant-integrity overlap: § "Composition authority stays on the controller" states the design's core capability-boundary invariant as "[the guest] cannot even enumerate [stages] beyond what allowedOrigins()-style inspection exposes," and the facet table enforces that by keying inspectPipeline() to controller. The design's own state/identity modeling (which data lives behind which facet) makes the budgeting advice it gives the guest un-executable by that guest. This isn't a taste call — a guest that follows the documented advice to multiply estimateCost(req) * maxAttempts has no method call that returns maxAttempts. Should-fix by picking one of: (a) fold the attempt multiplier into a client-facing estimateCost variant or a new client-facet maxAttempts getter, or (b) state explicitly that this budgeting math is something only the host (controller-holder) can perform and the guest must receive it out-of-band, and drop "a guest sizing a budget... must" language. [proposed-rule: a design that splits a capability into disjoint facets must cross-check every "how a guest does X" passage against the facet table — the verb named must actually sit on the facet the guest is said to hold.]

Notes (out of scope but worth flagging):

  • The response-side reuse of maxResponseBytes as both DoS-truncation ceiling and reservation ceiling (§ 1) looks like state/policy complecting at first read, but is essential rather than accidental: the design has no other knowable worst-case bound for an as-yet-unread response, so the two roles are structurally forced to share one number. Not a finding. [rule: n/a]

Self-improvement: no durable lesson beyond this engagement — the design was disciplined enough (explicit alternatives-considered, explicit accepted-risk callouts) that the categories mostly tested clean; the one real finding was a facet/prose cross-check, not a Hickey-lens complecting of state/identity/time. Noting for future decomplector passes on capability-split designs: always diff the facet table against every "the guest does X" sentence — that's a cheap, high-signal check this design almost passed.

ergonomist

Confirmed path. Here's the per-juror block.

ergonomist

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md § Exo and CLI surface additions defines set-request-bytes <name> --max <n> for setControllerMaxRequestBytes, which the same section calls "the request-side sibling of the Phase 1 maxResponseBytes." But the Phase 1 sibling (designs/cli-http-client.md line 121) is endo http set-bytes <name> <max-bytes> — positional, no flag, and not spelled set-response-bytes. Two operations the design itself declares siblings now spell with different verbs (set-bytes vs set-request-bytes) and different argument styles (positional vs --max flag). A user who has learned set-bytes for the response cap has no way to predict set-request-bytes --max for its declared sibling. [rule: roles/jurors/ergonomist/AGENT.md]
  • Same section's set-rate <name> --per-minute <n> [--controller-per-minute <n>] silently changes the call shape of the already-approved Phase 1 verb (cli-http-client.md line 120: endo http set-rate <name> <max-per-minute>, positional) to a flag-based form, with no migration note or callout that this is a breaking change to a shipped surface. This also breaks internal sibling coherence: set-bytes/set-time stay positional (unchanged, per the same table), so set-rate now spells differently from its own Phase 1 siblings for no stated reason. The design could add the optional aggregate as set-rate <name> <max-per-minute> [--controller-per-minute <n>], preserving both backward compatibility and positional-sibling consistency. [rule: roles/jurors/ergonomist/AGENT.md]
  • designs/http-adapter-pipeline.md § Exo and CLI surface additions: of the seven new/changed controller verbs, only two (setControllerMaxRequestBytes, setControllerMaxRequestsPerMinute) carry a Controller infix; setMeterPrice, setRetry, setBreaker do not, though all seven live on the same controller facet (per the method table's own Facet column). setControllerMaxRequestsPerMinute's infix is justified — it disambiguates from the existing per-client setMaxRequestsPerMinute. setControllerMaxRequestBytes has no such per-client sibling anywhere in either doc, so the infix is unearned there; it should read setMaxRequestBytes to match setMeterPrice/setRetry/setBreaker and its true sibling setMaxResponseBytes. [proposed-rule: sibling controller-facet setters spell without a facet-name infix unless disambiguating a same-named method at a different scope.]

Notes (out of scope but worth flagging):

  • § Exo and CLI surface additions argues inspect --pipeline "is a method selector, not a mode of one call," but nothing in the CLI syntax communicates that distinction to a user — a flag on inspect reads as a mode regardless of the backing dispatch. Not hostile enough to block; a should-fix if the panel wants CLI-level parity with the stated API split (e.g. inspect-pipeline). [rule: roles/jurors/ergonomist/AGENT.md]

Self-improvement: none — the two CLI-drift findings were caught by diffing this design's verb table against the already-approved cli-http-client.md verb table line-by-line; no gap in my own procedure to record.

copyeditor

copyeditor

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:398 states plainly "the fees concern is not a pipeline stage at all" (the meter stage draws on the charge account instead, giving "four non-transport stages"), but three other places in the same document call it a stage anyway: designs/http-adapter-pipeline.md:593 ("The fee stage draws against an attenuated charge account..."), the Relationship table ("a fee stage draws against" the ResourceLedger), and the Dependencies table ("ResourceLedger / purse the fee stage draws against"). A reader who reaches § 2 right after the explicit "not a pipeline stage at all" claim hits a heading and lead sentence that reintroduce the very stage the prior paragraph disclaimed, with no bridging note ("the meter stage, which the fees concern configures"). This is a real self-contradiction, not just loose phrasing, and it's repeated three times rather than a one-off slip. Reword every "fee stage" occurrence to "the meter stage (fee-side)" or similar, or drop the "not a pipeline stage at all" framing if "fee stage" is actually intended as shorthand for the meter stage's fee-drawing behavior. [rule: roles/jurors/copyeditor/AGENT.md]

  • designs/http-adapter-pipeline.md:29-34: "The maintainer's approval of Phase 1 asked for a follow-up that elaborates the controller/client system to support metering, fees, rate limiting, retries, and circuit breaking (error-based), mining the substantial prior art on HTTP adapter pipelines ... for design precedents, "recalling that these are pass-style interfaces."" Two issues compound: (1) the participle "mining" dangles — its grammatical subject is "The maintainer's approval," but the maintainer's approval isn't the one mining prior art; the follow-up (or its author) is. (2) The closing quoted fragment is spliced on after a bare comma with no framing verb ("as the maintainer put it," "noting that"), so the sentence trails into an unintegrated quotation. A reader has to reparse the whole sentence to recover who's doing what. Suggest: "This design elaborates the controller/client system to support metering, fees, rate limiting, retries, and circuit breaking (error-based), mining the substantial prior art on HTTP adapter pipelines ... for design precedents, per the maintainer's request to recall "that these are pass-style interfaces."" [rule: roles/jurors/copyeditor/AGENT.md]

  • designs/http-adapter-pipeline.md:375-376: "...then aborts the storm on the very next attempt). Third (the low-impact one) the pre-flight checks origin before method/header validation, where http-confine validates..." — missing comma after the parenthetical: "Third (the low-impact one) the pre-flight checks" reads as a run-on/comma-splice where "(the low-impact one)" and "the pre-flight checks" collide with no separator. Should be "Third (the low-impact one), the pre-flight checks...", matching the "First, ... Second, ..." pattern the same paragraph already establishes. [rule: roles/jurors/copyeditor/AGENT.md]

  • designs/README.md (Totals paragraph, new 2026-08-15 sentence): "Proposed 26 → 27, total 151 → 152." introduces new prose using the arrow glyph (U+2192), a typist-hostile code point per the standing rule — not exempt as a verbatim quote, fenced output, or single-glyph code span. Spell as 26 -> 27, total 151 -> 152 (matching the ASCII convention the rule requires; note the rest of the same paragraph, unchanged by this PR, already carries pre-existing uses that predate this rule's adoption and are not this PR's to fix, but the new sentence added here should not extend that pattern). [rule: skills/typist-friendly-code-points/SKILL.md]

Notes (out of scope but worth flagging):

  • The document is otherwise long but internally consistent in voice (present tense throughout) and its section transitions (e.g. "## The five concerns" opening by explicitly referencing "the table above") are well-built; no broader flow issues found in the ~1200-line design body beyond the items above. [rule: roles/jurors/copyeditor/AGENT.md]

Self-improvement: none this round — the brief's guidance (primary surface vs. typist-friendly-code-points backstop) was sufficient to catch both a genuine cross-section terminology contradiction and a mechanical glyph slip; no gap in the operating brief to report.

pedant

Confirmed. Here is the per-juror block.


Juror: pedant

Verdict: request-changes

Findings:

  1. [should-fix] designs/README.md:1458 (the new http-adapter-pipeline estimate row this PR adds) uses (U+2194 LEFT RIGHT ARROW) in "the byte-cap↔metering reconciliation." Project style requires the ASCII spelling <-> for this glyph. [rule: skills/typist-friendly-code-points/SKILL.md] (replacements table, row <->). This is new content, not carried-forward text, so it isn't grandfathered the way the pre-existing occurrences elsewhere in the same file (e.g. line 1055) are.

  2. [should-fix] designs/README.md, the **Totals:** paragraph (the line this PR edits to add the 2026-08-15 entry) introduces new prose reading "Proposed 26 → 27, total 151 → 152" using (U+2192) rather than ->. [rule: skills/typist-friendly-code-points/SKILL.md] Per that skill, "existing prose is fixed on encounter... when a role edits a file for another reason, it rewrites the typist-hostile glyphs in that file as part of the change" — this PR touches this exact line to add the new sentence, so the pre-existing occurrences in the same line, plus the two new ones it introduces, should be swept to -> while the line is being edited.

  3. [comment-only] designs/http-adapter-pipeline.md:1136, in § Open questions: "Default proposed: settle once at release, bill worst-case if never released." Elsewhere in the same document "worst case" is consistently left unhyphenated when used as a noun/object rather than a premodifier (e.g. line 106 "bill actual after," line 216 "worst case, not the whole retried call," line 541 "the reserved worst case"), and hyphenated only when it premodifies a following noun ("worst-case cost," "worst-case payload"). Here "worst-case" has no following noun (parallel to "bill actual"), so for internal consistency it should read "bill worst case." [rule: roles/jurors/pedant/AGENT.md § Operating norms, hyphenation of compound modifiers before vs. after the noun]

Checked and cleared: no em-dashes in the new/changed prose (skills/em-dash-style/SKILL.md); the new mermaid node/edge (hpipe) uses proper --> syntax, no ASCII-art or arrow-glyph diagram; no banner comment lines in the new file's code blocks (skills/no-comment-banners/SKILL.md); serial comma applied consistently across every "metering, fees, rate limiting, retries, and circuit breaking" list; all cross-document links are relative (skills/relative-paths/SKILL.md); the ## What is the Problem Being Solved? / ## Alternatives Considered Title-Case headings match the established boilerplate in the parent document designs/cli-http-client.md, not an inconsistency.

Self-improvement: none — the operating brief's guidance (layered project rules, hyphenation-as-premodifier, sibling-document convention checks) was sufficient to find and rule out findings confidently this pass.

novice

novice

Verdict: comment-only

Findings:

  • designs/http-adapter-pipeline.md § "1. Metering" ("Reserve, perform, settle" step 3, and the MeterReservationInterface.settle sketch): measurementId is used as a load-bearing idempotency key ("Idempotent on measurementId — a retried settle returns the receipt") but, unlike its sibling operationId two paragraphs earlier — which gets an explicit derivation (`${requestId}:${context.attempt}`) and a paragraph explaining why that derivation is safe — measurementId is never derived, sourced, or explained anywhere in the document. A reader who just built the mental model "every per-operation id in this design gets a stated derivation" (from operationId) hits measurementId a few lines later and cannot tell whether it is minted by the transport stage, equals operationId, or is something else entirely. This is exactly the "term is technically used but the mental model isn't built" gap. [rule: roles/jurors/novice/AGENT.md]
  • Same section, doc-wide pattern: every capability/interface the design introduces gets a M.interface(...) code sketch (HttpClientInterface, HttpStageInterface, ChargeAccountInterface, MeterReservationInterface) — except PriceSchedule, which is used repeatedly (setMeterPrice(price), "Pricing is a versioned PriceSchedule the ledger selects", the price.perByteRequest / perByteResponse / perMillisecond / perRequest fields in the cost formula) but never gets the same treatment. By the time the reader reaches the cost formula, they've been trained by four prior sketches to expect a fifth, and its absence is a real gap right at the document's central pricing mechanism, not a stylistic quibble. [proposed-rule: every capability/record type referenced by name more than once in a design should get the same code-sketch treatment as its siblings, or an explicit note that its shape is deferred]

Notes (out of scope but worth flagging):

  • "computrons" (§ 1, citing daemon-xs-worker-metering) is used with no inline gloss; a reader who hasn't read that sibling design has to guess it means "a unit of XS engine computation." Low cost since it's a linked, cited document, but worth a three-word parenthetical. [rule: roles/jurors/novice/AGENT.md]

Self-improvement: none — the novice brief's categories (a)-(e) mapped cleanly onto this design; no gap in the brief itself surfaced this round.

model claude-opus-4-8 · harness claude · garden 4e95097a

- estimateCost: document that the probe reflects only the funds axis, not
  the breaker/rate admission gates (critic); budget sizing that multiplies
  by maxAttempts belongs to the controller-holder since inspectPipeline()
  is controller-only, not the guest (skeptic/decomplector).
- attachChargeAccount table row: reword to gate on controller-holder
  access, matching the 'structural, not a provenance test' prose (critic).
- Retry idempotency: correct the false 'Phase 1 default is GET/HEAD'
  claim -- CONFINED_ALLOWED_METHODS is the full seven-method set (skeptic).
- Request contentLength: specify and test the omitted-contentLength case
  as an enforced maxRequestBytes cap, not an unbounded stream (skeptic).
- CLI verbs: set-request-bytes and set-rate keep the Phase 1 positional
  argument shape; rename setControllerMaxRequestBytes -> setMaxRequestBytes
  (ergonomist).
- Copy: fix 'fee stage' vs 'not a pipeline stage at all' contradiction,
  dangling 'mining' participle, missing comma, worst-case hyphenation.
- Glyphs: sweep -> and <-> to ASCII on the edited README lines.
- Add measurementId derivation note (novice).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel verdict (gauntlet round 5) — must-fix

Design panel, single-round. Disposition: must-fix (6 of 7 seats request changes; base ref 9d86783c0, head 750ed9d2).

critic

Per-juror block — critic — PR #992 (kriscendobot/endo-but-for-bots)

Verdict: request-changes

Findings:

  1. [should-fix] The costMax/costActual formulas mix bigint and number operands, which throws in JavaScript rather than computing a cost. designs/http-adapter-pipeline.md § "1. Metering" (both the costMax sketch and the costActual sketch in § Reserve, perform, settle, step 3) write price.perByteRequest * request.contentLength, price.perByteResponse * maxResponseBytes/bytesRead, and price.perMillisecond * effectiveDeadlineMs/min(elapsedMs, effectiveDeadlineMs). The design's own § "Numeric domain" paragraph states price.* terms are bigint (unbounded monetary quantities) while contentLength, bytesRead, elapsedMs, maxResponseBytes stay number (FiniteNonNegative), then asserts "the cost formula multiplies a bigint price-per-unit by an integer quantity" as if this is unproblematic. It is not: 5n * 3 throws TypeError: Cannot mix BigInt and other types, use explicit conversions. A builder implementing this formula literally as sketched hits a hard runtime error on the very first metered request. Worse, the fix isn't a bare BigInt(x) wrap — elapsedMs and (in principle) contentLength/bytesRead are not guaranteed integers under FiniteNonNegative (which only excludes NaN/±Infinity/negatives, not fractional values), and BigInt() throws RangeError on a non-integer. The design needs an explicit rounding rule (and it matters which direction: reserving costMax should round up to preserve the pessimal-case-refuses-first-before-headers invariant; whether costActual's settlement should also round up or down is a real billing-fairness choice the design currently elides) before the formula is implementation-ready. This is exactly the "will the implementation as sketched actually work" question the critic seat owns, and it sits in the design's most load-bearing section — the reservation invariant (costActual <= costMax) that every downstream refuse-before-read guarantee rests on. [rule: AGENTS.md § Numeric domain]

  2. [comment-only] "Incentive-correct" mislabels who is being incentivized in the retry-billing rationale. § "4. Retries" and the closing paragraph of § "1. Metering" both justify billing a guest for every retried attempt (3 attempts = 3 reservations/settlements, including attempts that fail purely on origin-side 5xx/timeout) as "the correct incentive (retries are not free)." An incentive only functions on the party that controls the behavior it prices, but maxAttempts/backoff is controller policy the guest cannot see or set (inspectPipeline() is controller-only per § "Exo and CLI surface additions"). The framing is cost-attribution-correct (the design already gets this right operationally: estimateCost deliberately excludes the multiplier, and the host must pass a pre-sized budget out of band), but calling it an "incentive" overstates what the mechanism does to the party actually paying it. Recommend rewording to "attributes real resource cost" rather than "incentive," since the rationale-vs-reality gap here is small but the term is doing more work than the mechanism supports.

Self-improvement: No update to roles/jurors/critic/AGENT.md proposed this round; the numeric-domain formula check (bigint/number arithmetic mixing in cost sketches) is a useful pattern to fold into future reviews of any metering/billing design, but it's an instance of the existing AGENTS.md rule rather than a gap in the critic brief itself.

skeptic

skeptic

Verdict: request-changes

Findings:

  • The retry stage's eligibility test rests on an undefined mechanism. § 4 says retry fires for "GET, HEAD, and any method the controller's closed set marks idempotent," but the cited substrate (http-confine.js's allowedMethods: Set<string>, and CONFINED_ALLOWED_METHODS) carries no per-method idempotency annotation — it is a flat set of strings, and this design's own § Exo and CLI surface additions introduces no verb to mark one. PUT and DELETE are canonically idempotent under HTTP semantics and sit exactly in this unspecified gap: nothing in the doc says whether they retry. Either name the actual mechanism (a new marked set, or hard-code GET/HEAD only) or the Phase 3.6 builder has no spec to implement against. [proposed-rule: a design that gates behavior on a classification of an existing closed set ("the controller's closed set marks idempotent") must define where that marking is stored, or the classification is unimplementable as written.]

  • Rationale mismatch (secondary surface, § 4): "the cost of a retry is honestly billed... which is the correct incentive (retries are not free)" assumes the guest controls whether a retry happens. It doesn't — setRetry is controller policy, not a per-request guest choice. So charging a non-refundable perRequest admission fee for each of, say, two attempts that failed purely because the origin was unhealthy (5xx/timeout — not the guest's doing) doesn't incentivize the guest toward anything; it just makes origin flakiness the guest's expense. Either the guest needs a lever (opt out of retry per-request) or the rationale should say plainly "the guest bears origin-flakiness cost as the price of automatic retry," not "honest billing/correct incentive." [proposed-rule: a cost-model rationale that invokes "incentive" for a party must confirm that party has a choice the pricing is meant to shape.]

  • Test-catalog gap: § Exo and CLI surface additions asserts a specific security property — a client-held ref can never reach attachChargeAccount because "the client facet has no verb that returns or accepts an account" — but the Test plan has no item asserting this. Given the doc calls this out as "structural, not a provenance test," it's exactly the kind of claim that should be pinned by a listed test (assert HttpClientInterface/HttpStageInterface never expose an account-shaped argument or return). [proposed-rule: a design's explicit "X is structurally unreachable" capability claim needs a corresponding Test plan item.]

Notes (out of scope but worth flagging):

  • The Test plan's "Cross-boundary" bullet exercises a "FeePurse" — a type name introduced nowhere else in the doc, which otherwise only defines ChargeAccount/MeterReservation. Likely a stale name from an earlier draft; reconcile before build. [proposed-rule: a design's Test plan must use only type names the doc itself defines.]

Self-improvement: none this round — the brief's categories (a)-(e) were sufficient to reach should-fix findings without needing a new juror-facing tool or hint change.

decomplector

decomplector — PR #992 (kriscendobot/endo-but-for-bots), designs/http-adapter-pipeline.md

Verdict: request-changes

Findings:

  • Price snapshot vs. mutable current-price state is asserted, not modeled. § "The cost function" states registering a new PriceSchedule version via setMeterPrice "does not disturb reservations already open against the prior version," but the design never says how a reservation is bound to the price version that computed its costMax. settle()'s costActual formula (§ 1, step 3) re-derives from price.* with no stated source — live controller policy or a snapshot captured at reserve(). The design elsewhere is disciplined about exactly this distinction (StageContext is explicit that "the meter's live reservation lives in the meter stage's own closure, captured at composition, like next... never in this shared record"); the same "capture at construction, not re-read live" pattern is needed for price but is left implicit. Without it, a setMeterPrice call racing an in-flight reservation either violates the stated invariant or relies on an unstated implementation convention a builder must guess. [proposed-rule: a design that asserts a state-mutation-immunity invariant (a config change must not retroactively affect an in-flight operation) must name the mechanism — snapshot-at-start, versioned reference, etc. — not just the invariant.]

  • FiniteNonNegative conflates continuous and discrete value domains. § "Numeric domain" splits monetary quantities (bigint) from per-request measures (number, guarded FiniteNonNegative) and states the cost formula "multiplies a bigint price-per-unit by an integer quantity" — but FiniteNonNegative (M.number() + gte(0) + not(Infinity) + not(NaN)) admits fractional values (0.5 passes), and elapsedMs/bytesRead/contentLength all reuse it (MeasureShape, RequestShape.contentLength). bigint * number throws in JS when the number isn't an integer, so the "integer quantity" claim the arithmetic depends on is asserted in prose but not enforced by the guard the design specifies. This is the classic braided-primitive case the seat's category (a) names: one shape (FiniteNonNegative) is reused for two different value domains (a continuous budget/price term and a discrete measured count) that need different guards. [proposed-rule: a numeric guard reused across a continuous-value field and a count field that feeds bigint arithmetic must be two distinct shapes, not one.]

Both are should-fix: naming the price-snapshot mechanism and splitting an IntegerNonNegative guard from FiniteNonNegative are revisions to the doc, not rework of the pipeline shape. The pass-style stage/context split, the pre-flight/onion separation, and the value-vs-capability split in StageContext are otherwise exemplary decomplecting — no further categories warranted a finding.

Self-improvement: none this round — the brief's category walk applied cleanly to a design of this size.

ergonomist

ergonomist

Verdict: request-changes

Findings:

  • The new setMaxRequestBytes verb mirrors Phase 1's setMaxResponseBytes in name and table position (designs/http-adapter-pipeline.md:901, set-request-bytes CLI at designs/http-adapter-pipeline.md:933-941), but the two enforce differently at the boundary: the response cap truncates (a DoS defense over untrusted inbound data), while the request cap rejects with a structured error (designs/http-adapter-pipeline.md:470-508, since silently truncating caller-authored outbound data would corrupt it). A caller who has learned "byte caps truncate" from the shipped set-bytes/setMaxResponseBytes convention has no naming cue that its sibling instead throws. The asymmetry is well-reasoned in prose but invisible at the surface (verb name, table entry, CLI help text) where a user actually decides how to handle the cap. Fix: name the request-side verb to signal the different failure mode (e.g. setMaxRequestBytes documented inline in the verb table with "(rejects, does not truncate)"), or add a one-line CLI help string calling out the asymmetry, so the naming symmetry doesn't imply behavioral symmetry it doesn't have. [proposed-rule: a setter that names a "cap" alongside a same-shaped sibling cap must surface a discoverable cue (name suffix, inline table note, or help text) whenever the two caps fail differently (truncate vs. reject).]

  • Error visibility is inconsistent across the three structured errors this design introduces. InsufficientFundsError gets an explicit shape ({ required, available }, designs/http-adapter-pipeline.md:701), matching the document's own rigor elsewhere (MeasureShape, StageContextShape, FiniteNonNegative are all spelled out). But CircuitOpenError and RateLimitError — each referenced 5+ times, including as things a caller is explicitly told to "distinguish" (designs/http-adapter-pipeline.md:704-721) and that the test plan asserts rejections against by name (designs/http-adapter-pipeline.md:1165-1173) — never get a shape or a common discriminant convention (no name/code field, no instanceof contract). An implementer has three sibling error types and only one specified shape; the other two are free-floating names with no spec to build against, and a caller has no documented way to pattern-match "which of the three fired." Fix: give CircuitOpenError/RateLimitError the same shape treatment as InsufficientFundsError, or state the shared discriminant (e.g. all three are makeError instances distinguishable by error.message tag or a common .name) once, so error-visibility is coherent across the surface rather than one-third specified. [rule: designs/http-adapter-pipeline.md's own precedent of spelling every cross-cutting shape (MeasureShape, StageContextShape) rather than leaving it implicit]

Notes (out of scope but worth flagging):

  • estimateCost's single-attempt-only quote (designs/http-adapter-pipeline.md:723-739) is a real mental-model gap for a guest ("estimateCost" reads as "what this call will cost," not "what one attempt costs"), but the design already documents the limitation at length and gives a coherent reason it can't fold in maxAttempts. Comment-only: worth a stronger name (estimateAttemptCost) if this API ships as-is, but not a blocker at design stage. [proposed-rule: a cost/estimate method whose scope is narrower than its call-site name implies should name that scope explicitly rather than relying on prose to correct the reader's first impression.]

Self-improvement: none this round — the brief and panel-review skill matched the task cleanly; no gap surfaced worth encoding.

copyeditor

Per-juror block — copyeditor — PR #992 (kriscendobot/endo-but-for-bots)

Verdict: comment-only

Findings:

  1. should-fixdesigns/README.md, new link-list entry (+[http-adapter-pipeline]...): "follow-up to the PR #286 endo http mk Phase 1 approval that elaborates the controller/client pair into a pass-style middleware pipeline..." The relative clause that elaborates attaches, by standard nearest-noun attachment, to "approval" rather than to "follow-up" — on a literal read it says the approval elaborates the controller/client pair, when the doc's own body makes clear the approval only requested the elaboration and this new design performs it. Reword to break the attachment, e.g.: "...added 2026-08-15 as a follow-up to the PR #286 endo http mk Phase 1 approval; it elaborates the controller/client pair into a pass-style middleware pipeline staging metering, fees, rate limiting, retries, and circuit breaking." [rule: roles/jurors/copyeditor/AGENT.md § Operating norms (sentence structure / be specific)]

  2. should-fixdesigns/http-adapter-pipeline.md, FiniteNonNegative code comment: "a NaN deadline disables the retry and timeout stops (a remaining <= 0 test against NaN is always false)." "stops" reads ambiguously as either a shared plural noun ("the retry-stop and timeout-stop [checks]") or a verb, giving a momentary garden-path parse ("disables the retry, and [then] timeout stops"). Reword to remove the ambiguity, e.g. "disables both the retry-stop and timeout-stop checks." [rule: roles/jurors/copyeditor/AGENT.md § Operating norms (sentence structure that obscures meaning)]

  3. should-fixdesigns/http-adapter-pipeline.md, § 1 Metering, "Measurement happens at the resource boundary" paragraph: "the meter stage trusts the bytesRead reported by the far-ref transport stage it composed (a capability it endowed)." The pronoun it (antecedent: meter stage) is given the composer's/endower's role here, but § "Composition authority stays on the controller" states composition and endowment are done by the controller's composer, not by each stage itself — a stage is endowed with its next ref, it doesn't endow one. As written the voice is inverted and the antecedent reads backward from the rest of the document. Suggest: "...reported by the far-ref transport stage it was composed with (a capability it was endowed with at composition time)." [rule: roles/jurors/copyeditor/AGENT.md § Operating norms (pronoun antecedents)]

  4. should-fixdesigns/http-adapter-pipeline.md, § 4 Retries, "Retryable classes": "429 is retryable here (backing off and re-sending your own request is the correct response to your own throttle)". This is the only second-person address in the entire document; every other passage uses third person ("a caller", "the guest", "a host"). The sudden "your"/"your own" breaks voice consistency. Reword in third person, e.g. "backing off and re-sending is the correct response when a caller is throttling itself." [rule: roles/jurors/copyeditor/AGENT.md § Operating norms (voice and tense consistency)]

Note (not a finding): the diff's edit to the **Totals:** historical paragraph in designs/README.md converts its prior arrows to ASCII -> throughout — good compliance with typist-friendly-code-points; no action needed.

Self-improvement: none — the prose issues found were ordinary sentence-level tangles/voice slips in an otherwise very long, dense document; no new standing rule is warranted from this pass.

pedant

pedant

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md mixes heading case at the ## level without a stated reason. Fourteen ## headings are sentence case (## Relationship to existing designs, ## Canonical stage order, ## The five concerns, ## Exo and CLI surface additions, ## SSRF / DoS posture is preserved, ## Test plan, ## Open questions, …) but two are title case: ## What is the Problem Being Solved? and ## Alternatives Considered. Pick one case convention for ## headings and apply it to all fourteen; the ### level is already internally consistent (all sentence case) and can stay as the model. [rule: roles/jurors/pedant/AGENT.md § Operating norms ("capitalization ... title case vs sentence case in headings used consistently")]

  • designs/http-adapter-pipeline.md line ~230: "entirely (e.g. a fee purse held by the gateway), because every hop is a" uses the Latin abbreviation e.g. The project's style rule forbids Latin shorthand in bot-authored prose (this design's front matter marks it "Kris Kowal (prompted)" — bot-authored, not the maintainer's own writing, so the exemption in that skill's § Scope does not apply). Rewrite as "for example, a fee purse held by the gateway" per the skill's own worked example (Foo, e.g. bar.Foo (for example, bar).). This is the only Latin-shorthand instance in the new file; the rest of the ~1250-line document is clean. [rule: skills/no-latin-shorthand/SKILL.md]

Notes (out of scope but worth flagging):

  • designs/README.md's long "Totals" prose paragraph is fully replaced by this diff (a new sentence is prepended and the arrow separators in the surrounding, pre-existing sentences were mechanically converted to ->), yet the em-dashes already present in that same carried-over prose (e.g., "...consolidation below." / "...and folds in...") were left as em-dashes, so the paragraph is now inconsistent between its arrow spelling (fixed) and its dash spelling (not fixed). This text predates the PR and isn't newly authored, so I'm not calling it must-fix, but if this paragraph is touched again it's worth sweeping in the same pass. [rule: skills/em-dash-style/SKILL.md]

The new design document itself (designs/http-adapter-pipeline.md) has zero em-dashes, zero curly quotes/ellipses/arrow glyphs, no ASCII diagrams needing mermaid conversion, and consistent serial-comma/hyphenation/relative-link discipline throughout — it reads as edited, not raw, apart from the two items above.

Self-improvement: no skill or role gap surfaced this round; both findings map cleanly to existing standing rules.

novice

novice — PR #992 (kriscendobot/endo-but-for-bots), designs/http-adapter-pipeline.md et al.

Verdict: request-changes

Findings:

  • should-fix. The reorder rationale paragraph (designs/http-adapter-pipeline.md § Canonical stage order, the ~28-line block right after the stage table) packs three distinct claims — origin-check-before-rate-token, breaker's position relative to retry, and the origin-vs-method/header precedence swap — plus a genuine behavior-change callout ("one narrow observable does change") into one unbroken paragraph with only inline "First/Second/Third" markers. A reader has to hold three separate claims in suspension to find which one the behavior-change note actually attaches to (it's the "Third" one). Splitting into a numbered list would let the reader track each claim, and its caveat, independently. [rule: skills/gricean-maxims/SKILL.md § Manner: be clear and orderly]

  • should-fix. § 1 Metering gives the costMax formula and the reserve/perform/settle state machine entirely in the abstract — no worked pass with concrete numbers (e.g., a declared contentLength, a price schedule, and the resulting costMax/costActual). Each individual rule (reserve, settle, release) is separately clear, but nothing lets a first-time reader check their running mental model against a real number before the design moves on to § 2 Fees. [proposed-rule: a design's central cost/state-machine formula includes at least one concrete worked-numbers example, not the formula alone]

  • should-fix. "Pass-style" is used unglossed in the title and in the opening two paragraphs of § What is the Problem Being Solved (citing the maintainer's "recalling that these are pass-style interfaces") roughly 40 lines before its first actual explanation (observation 5's exo-facet definition). A reader without prior Endo vocabulary carries an undefined key term through the whole problem statement. Move the exo-facet/pass-style gloss earlier, or add a one-clause gloss at first use. [rule: roles/jurors/novice/AGENT.md § Secondary surface]

  • comment-only. The stage-interface and ChargeAccount code blocks (~line 153–247, ~600s) interleave dense rationale as inline comments the reader must parse simultaneously with unfamiliar M.interface/M.splitRecord syntax. Separating "what the shape is" from "why" (rationale before/after the block, not woven through it line-by-line) would lower the parse cost for a reader new to the Pattern/Guard vocabulary. [proposed-rule: a design's illustrative code sketch keeps rationale in surrounding prose, not embedded as line-level comments the reader must interleave with syntax]

Notes (out of scope but worth flagging):

  • The design's overall problem→scope→prior-art→stage-order→five-concerns→staging chain is followable end to end; none of the above breaks the logical thread, they slow it. [rule: roles/jurors/novice/AGENT.md]

Self-improvement: roles/jurors/novice/AGENT.md — skills/gricean-maxims/SKILL.md § Manner (ordering / buried-decision) turned out to be a citable standing rule for novice's "logical progress"/"prose density" findings on design docs, and the role's ## Skills list doesn't currently name it. Not edited this run (out of scope for this invocation); worth adding on a future pass.

model claude-opus-4-8 · harness claude · garden 4e95097a

- Numeric domain: split IntegerNonNegative from FiniteNonNegative for
  discrete count fields; widen counts to bigint with BigInt() in the cost
  formulas (bigint*number throws); state the ceil rounding rule for the
  continuous deadline term so costActual <= costMax holds (critic,
  decomplector).
- Retry idempotency: name the mechanism -- a retry-stage idempotentMethods
  set (default GET/HEAD), distinct from CONFINED_ALLOWED_METHODS; PUT/DELETE
  opt-in (skeptic).
- Price immunity: name the snapshot-at-reserve mechanism (decomplector).
- Structured errors: spell CircuitOpenError/RateLimitError shapes and a
  shared name discriminant alongside InsufficientFundsError (ergonomist).
- setMaxRequestBytes: surface the reject-vs-truncate asymmetry at the verb
  table and CLI help (ergonomist).
- Retry billing rationale: reword incentive -> cost-attribution (critic,
  skeptic).
- Test plan: add structural-unreachability test for attachChargeAccount,
  integer-guard and error-discriminant tests; fix stale FeePurse name to
  ChargeAccount (skeptic).
- novice: reorder rationale as a numbered list, add a worked-numbers
  metering example, gloss pass-style at first use.
- copyeditor: fix README attachment, ambiguous 'stops', inverted composer
  pronoun, second-person voice.
- pedant: sentence-case two headings, remove Latin e.g.
- Keep the doc free of em-dashes/arrow glyphs (typist-friendly code points).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@kriscendobot kriscendobot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Panel disposition: must-fix (posted as a comment review; GitHub disallows request-changes on ones own PR).

Panel review — round 6 (design/code panel, single-round)

Disposition: must-fix — 15 request-changes, 7 comment-only, 6 approve across the seats below.

Note: several seats flagged that this worktree's local llm ref is ~3 weeks stale, so a literal llm...HEAD diff pulls in ~493 already-merged commits/3400+ unrelated files. PR #992 itself is design-doc-only (designs/http-adapter-pipeline.md plus small designs/README.md/designs/cli-http-client.md registration edits). Findings scoped to that design doc are the authoritative must-fix set (assessor, typist, stylist, packager, corner-prober, fast-checker, changeset-auditor, surfacer); findings about packages/reminder, capn-web, thixotrope, ironhorse, etc. arise from the stale cross-diff and are out of scope for this PR.


assessor

Assessor review — PR #992 (kriscendobot/endo-but-for-bots)

Reviewed diff: git -C .../project-wt-endojs-endo-but-for-bots-pr992-gauntlet-panel-6-fb075216 diff llm...HEAD resolved, after tracing the base, to the true PR content — the local llm ref in this worktree is ~3 weeks stale (67dfc18b, 2026-07-22) versus upstream/llm (071b102f, 2026-08-15), so a literal llm...HEAD pulls in 493 unrelated commits/3423 files. The actual PR is the design-http-adapter-pipeline branch: designs/http-adapter-pipeline.md (new, 1408 lines) plus small designs/README.md / designs/cli-http-client.md registration edits. This is a design document, not implementation code; I reviewed its formulas and control-flow claims as spec.

Verdict: request-changes

Findings:

  1. Must-fix — the cost formula's deadline units contradict its own declared type and worked example. CallerContextShape.deadline is documented as // absolute ms; the wall-clock budget proposal (designs/http-adapter-pipeline.md:207), and pre-flight step 5 rejects a deadline that is not "still in the future against the injected now" (designs/http-adapter-pipeline.md:363-371) — both confirm deadline is an absolute timestamp. But the cost formula computes effectiveDeadlineMs = min(deadline ?? timeoutMs, timeoutMs) (designs/http-adapter-pipeline.md:489) with no subtraction of now anywhere. Applied to an absolute epoch-ms deadline, this min() is always timeoutMs (epoch ms dwarfs any timeoutMs), so a caller deadline can never lower costMax — directly contradicting the worked example two paragraphs later ("deadline = now + 5_000... min(5_000, 30_000) = 5_000", designs/http-adapter-pipeline.md:510-512), which only works if deadline were a duration. The § Open questions resolution explicitly publishes the invariant "a shorter caller deadline lowers costMax" (designs/http-adapter-pipeline.md:1369-1379) as settled design; the formula as written falsifies it on its own example. [rule: roles/jurors/assessor/AGENT.md] (invariant-claim overlap: "the code already falsifies the published invariant; no attack needed"). Fix: either spell deadline as a duration everywhere (adjust the pre-flight "in the future" language) or fix the formula to effectiveDeadlineMs = min((deadline ?? (now + timeoutMs)) - now, timeoutMs) and correct the worked example accordingly.

  2. Should-fix — retry's error-classification table omits RateLimitError. § 4 "Retryable classes" (designs/http-adapter-pipeline.md:902-912) enumerates exactly what is/isn't retried (connection errors/timeouts/5xx/429 vs. other-4xx/pre-flight-rejection/InsufficientFunds) but never states whether a RateLimitError bubbling up from the rate stage (which sits inside retry, per the stage-order table) is treated as terminal or retryable. Since rate limiting and the meter are both "below" retry in the onion, this is a real control-flow gap an implementer must resolve without design guidance. [proposed-rule: a stage-pipeline design's retryable/non-retryable error enumeration must cover every structured-error type the design itself introduces, not only the ones from the concern being described in that section.]

Out of scope: the settle(reservation, measurementId, {...}) prose in § 1 (designs/http-adapter-pipeline.md:639) reads as a 3-arg free-function call but MeterReservationInterface.settle (designs/http-adapter-pipeline.md:714) is a 2-arg method on the reservation receiver — informal notation, not a contradiction; not filing as a finding.

Self-improvement: none — the brief and skills matched this task; the only friction was the stale local llm ref requiring a manual merge-base trace, which is environmental rather than a gap in the assessor's own instructions.

typist

Now I have the shape. Let me produce the per-juror block.

typist

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:522-524 types the new RequestShape.contentLength field as FiniteNonNegative ("this design adds contentLength: FiniteNonNegative to RequestShape"), but the document itself classifies contentLength as a discrete count requiring IntegerNonNegative, not the continuous FiniteNonNegative, in two other places: line 196-197 ("adds one optional field to RequestShape -- contentLength: IntegerNonNegative, ... an integer count, not a continuous budget") and line 571-576 (§ Numeric domain: "the per-request measures (contentLength, bytesRead, elapsedMs) ... stay number -- but the discrete IntegerNonNegative, not the continuous FiniteNonNegative"). FiniteNonNegative admits fractional values (e.g. 0.5); IntegerNonNegative does not. This is exactly the type-runtime drift the typist backstops: if an implementor follows the line-523 prose instead of the two correct declarations, a fractional contentLength would pass the boundary guard and then hit BigInt(0.5) -> RangeError in the cost formulas at lines 484 and 643 (price.perByteRequest * BigInt(request.contentLength)), exactly the failure mode § "Numeric domain" and the test plan (line 1245-1250, "Count fields reject fractional values") say the guard exists to prevent. Fix: change FiniteNonNegative to IntegerNonNegative at line 523 so all three mentions agree. [rule: roles/jurors/typist/AGENT.md § Type-runtime drift is the recurring typist finding]

Notes (out of scope but worth flagging):

  • None.

Self-improvement: no change to roles/jurors/typist/AGENT.md proposed this round — the existing "type-runtime drift" rule already covers this finding class (a self-contradictory type spec for the same field within one document) without needing a new rule.

stylist

Before the per-juror block: the literal git -C ... diff llm...HEAD command from the task produced a 3422-file, 183k-line diff — the worktree's local llm ref is stale (three weeks behind origin/llm), so the symmetric diff pulled in ~493 already-merged commits unrelated to this PR. I isolated PR #992's actual content by diffing against 9d86783c0 (the newest commit already carrying an upstream/llm-* tag, i.e. the true merge-base): a 5-file, design-doc-only change adding designs/http-adapter-pipeline.md plus two designs/README.md cross-reference edits. Reviewed that.

stylist

Verdict: request-changes

Findings:

  • The design's own illustrative code sketches and prose repeatedly name the request-record parameter/variable req — a bare abbreviation of request — e.g. the stage-body sketch request: async (req, cancellation, context) => { and its body E(this.state.next).request(req, cancellation, context) at designs/http-adapter-pipeline.md:313,315, and the same shorthand recurs at lines 147, 273, 359, 811, 832, 843, 1030, and 1282 (req.url, estimateCost(req), etc.). The maintainer's standing rule spells out identifiers in full even when the abbreviation is unambiguous, and reqrequest is one of the explicitly named cases. This design otherwise takes naming precision seriously (it dedicates a whole paragraph, lines 1082–1087, to spelling --per-byte-request/--per-byte-response out in full specifically to avoid ambiguity), which makes the pervasive req shorthand elsewhere in the same document an inconsistency worth fixing rather than a one-off slip. Fix: rename every req in the sketches and prose to request (the doc already uses request as the guarded-interface method name and the record's own field name, e.g. RequestShape, so this aligns the parameter name with the surrounding vocabulary instead of introducing a second, abbreviated one). Must-fix. [rule: roles/jurors/stylist/AGENT.md]

Notes (out of scope but worth flagging):

  • None — the doc explicitly disclaims (lines 100–102) that the concrete verb/exo names (setMeterPrice, attachChargeAccount, etc.) are placeholders pending a future namer dispatch, so no finding is raised against those pending a later naming pass.

Self-improvement: none — the abbreviation rule already exists and the finding was a straightforward application of it; no gap in the stylist brief surfaced.

packager

packager

Verdict: request-changes

Findings:

  • designs/README.md:1458, the milestone-table summary cell for http-adapter-pipeline, undercounts its own design's scope on two axes that were never revised across all five fix rounds (commits 43ff29d78..33d202fdf), even though those rounds substantially grew both the verb table and the concerns section:
    • "five new controller verbs" — the Exo/CLI surface table the same PR adds (designs/http-adapter-pipeline.md:1021-1030) lists seven controller-facet verbs (setMeterPrice, setMaxRequestBytes, attachChargeAccount, setControllerMaxRequestsPerMinute, setRetry, setBreaker, inspectPipeline); estimateCost is the sole client-facet addition. The summary was written in the initial commit (43ff29d78) when the verb set was smaller and never reconciled after round-3's setControllerMaxRequestBytes and round-4's rename to setMaxRequestBytes landed.
    • "four staged concerns (meter, rate, retry, breaker)" — the design's own ## The five concerns section (designs/http-adapter-pipeline.md:419) enumerates five: Metering, Fees, Rate limiting, Retries, Circuit breaking. The summary cell silently drops Fees, the ChargeAccount/purse-capability mechanism that the same cell separately name-checks ("the ChargeAccount/MeterReservation interfaces"), so the cell is internally inconsistent about whether fees are a staged concern.
      This is the packager's "changeset content" concern applied to a design-only PR: designs/README.md is this repo's changeset-equivalent summary of what the design actually specifies, and it currently misdescribes the surface it points readers to. must-fix. [rule: roles/jurors/packager/AGENT.md § changeset content]

Notes (out of scope but worth flagging):

  • Diff hygiene otherwise clean: all six commits touch only designs/README.md, designs/cli-http-client.md, and designs/http-adapter-pipeline.md; no stray code, generated-file, or yarn.lock churn; each round's commit body is specific about which juror seat(s) it addresses. No changeset is expected (design-doc-only PR, no packages/ touched). [rule: skills/changeset-discipline/SKILL.md]

Self-improvement: nothing this time — the finding traces to an existing brief section (§ changeset content); no new rule needed.

archivist

archivist

Verdict: approve

Findings:

No doc/comment-accuracy defects found. This PR's actual unique content (llm...HEAD against the design-http-adapter-pipeline branch pulls in ~3400 unrelated files because llm has diverged since this branch's fork point; the branch's own commits touch only designs/http-adapter-pipeline.md and designs/README.md, confirmed via git show llm:designs/http-adapter-pipeline.md → file absent on llm) is a new design document plus its required README sync.

Checked and clean:

  • All internal § cross-references resolve — both true ##/### headings (e.g. § Canonical stage order, § Exo and CLI surface additions, § Staging into the Phase plan) and the document's bold-label pseudo-headings (§ The cost function, § Numeric domain, § The client-to-stage seam, § Trip policy) all exist at the referenced location. [rule: roles/jurors/archivist/AGENT.md § Cross-document and intra-document section references resolve]
  • The one external cross-reference, gateway-package.md § Open Questions and § Feature 1 (Chat-hosting), both resolve to real headings in that file (## Open Questions at line 1433; ### Feature 1: Chat-hosting... at line 391). [rule: roles/jurors/archivist/AGENT.md § Cross-document and intra-document section references resolve]
  • Factual claims about existing code are accurate against the real source: http-confine's fixed step order (rate→method/header→origin→fetch→redirect→byte-cap) and CONFINED_ALLOWED_METHODS (GET, HEAD, POST, PUT, DELETE, OPTIONS, PATCH) both match packages/http-confine/src/http-confine.js exactly, including the specific reordering claims in § Canonical stage order (http-confine spends the rate token before the origin check; validates method/headers before origin — both verified line-by-line against the implementation).
  • Worked numeric example (§ 1, the costMax/costActual/release() arithmetic) checks out exactly by hand.
  • designs/README.md is correctly synchronized per designs/AGENTS.md § Progress Tracking: new summary-table row, dependency-graph node (hpipe), milestone-table entry, size/time estimate row, and the running totals prose all updated together. [rule: designs/AGENTS.md § Cross-document]
  • No banner-style comments introduced.
  • Metadata table conforms to designs/AGENTS.md (Created/Updated/Author/Status all present; Author: Kris Kowal (prompted) matches convention).

Notes (out of scope but worth flagging):

  • The stale-base diff blowup (493 commits, 3422 files between llm and this branch's HEAD) is a weaver/rebase concern, not a docs issue — flagging for the judge/supervisor so the panel isn't mistakenly run against the full noisy diff. [proposed-rule: when llm...HEAD for a dispatched PR review pulls in file counts wildly disproportionate to the PR's stated scope, the dispatching gardener should verify the branch is rebased onto current llm before fanning the panel, or scope juror dispatch to the PR's actual unique commit range.]

Self-improvement: none — no new standing rule needed beyond the proposed-rule above about stale-base diff blowup, which I'm forwarding rather than encoding myself since it's a dispatch/weaver-side concern outside the archivist's remit.

prover

prover

Verdict: comment-only

Findings:

  • None. This PR's actual content (verified via git diff <merge-base-with-upstream/llm>...HEAD, 6 commits / 4 files — see caveat below) is a design document (designs/http-adapter-pipeline.md) plus two designs/README.md/designs/cli-http-client.md cross-reference updates. There is no production code and no test code in the diff, so the prover's primary surface (does each new test fail on reverted production code) and secondary surface (correctness on the tested path) have nothing to check. [rule: skills/regression-evidence/SKILL.md] (scoped to "every new test"; none exist here).
  • Positive observation, not actionable: the design's own § Test plan already writes each item in load-bearing form ("assert the seam's call count is 0," "assert throws," "assert the balance moves once," etc.), naming the specific negative-space assertion that would catch the naive/broken implementation rather than a name-only placeholder ("cover metering"). This is exactly the standard skills/regression-evidence/SKILL.md asks a builder's eventual test suite to meet, stated up front. No fix needed; recorded for the fixer/builder who implements this plan later. [rule: skills/regression-evidence/SKILL.md]

Notes (out of scope but worth flagging):

  • Infrastructure caveat, not a PR defect: this worktree's local llm/origin/llm ref is stale (2026-07-22) against upstream/llm (2026-08-15). A literal git diff llm...HEAD in this checkout pulls in ~3400 unrelated files/493 commits from the fork's sync lag. I instead diffed against the merge-base with upstream/llm (9d86783c0), which cleanly isolates this branch's own 6 commits. Future jurors dispatched into this same worktree should do the same rather than trust the literal llm...HEAD diff at face value. [proposed-rule: when a jury seat's git diff llm...HEAD reports an implausibly large file/commit count for a PR's stated scope, cross-check git merge-base upstream/llm HEAD before reviewing, and note the discrepancy rather than reviewing the inflated diff.]

Self-improvement: no change needed to roles/jurors/prover/AGENT.md this round; the brief already correctly scopes the prover to test/production-code diffs, and a design-only PR simply has none. Worth folding the proposed-rule above into skills/worktree-per-pr/SKILL.md or the panel dispatch step so a stale fork llm ref doesn't silently hand every seat a 3400-file diff.

curator

curator

Verdict: comment-only

Findings:

  • designs/http-adapter-pipeline.md:711MeterReservationInterface is defined via M.interface('MeterReservation', {...}) but, unlike its three siblings in the same doc (HttpClientInterface line 246, HttpStageInterface line 267, ChargeAccountInterface line 758), it is never followed by a harden(...) call. Every other guard-bearing interface this design sketches is hardened immediately after definition; an unhardened interface object is mutable, and since this is a design doc that later gets implemented near-verbatim, the omission is likely to propagate into the real MeterReservationInterface export unless caught now. Should-fix: add harden(MeterReservationInterface); after line 724, for consistency with the doc's own established pattern. [rule: skills/panel-review/SKILL.md § Per-juror block shape (curator public-surface remit); pattern drawn from the doc's own three sibling interfaces at lines 246/267/758]
  • designs/http-adapter-pipeline.md:711 — the interface tag string is 'MeterReservation', breaking the Endo* tag-naming convention the doc otherwise uses consistently ('EndoHttpClient', 'EndoHttpStage', 'EndoChargeAccount'). Minor naming-shape inconsistency worth resolving before this design is used as an implementation template, though not blocking for a design-only PR. [proposed-rule: sibling exported interface guards introduced in the same design/module should share a naming prefix convention unless a deliberate exception is noted]

Notes (out of scope but worth flagging):

  • No changeset is present, which is correct: this PR touches only designs/*.md (no package code), so no bump is needed. [rule: skills/changeset-discipline/SKILL.md]
  • The design is careful to reuse the existing RequestShape/ResponseShape from cli-http-client.md by extension (contentLength added, not redeclared) rather than re-declaring the option shape — this is exactly the canonical-home/re-export discipline the curator looks for, done correctly. [rule: roles/jurors/curator/AGENT.md § Cross-package option types live in one canonical package and re-export]

Self-improvement: no change proposed to roles/jurors/curator/AGENT.md this round; the existing "public API surface" remit extended cleanly to a design-only PR's sketched interfaces (treating M.interface(...) blocks in a design doc as the proposed public surface). Worth a future one-line addition once a second design-panel curator dispatch confirms the pattern: "on a design-only PR, apply the same signature/shape/hardening-consistency check to any M.interface(...) sketches the doc proposes, since these designs are implemented near-verbatim."

migrator

migrator

Verdict: request-changes

Findings:

  • The design's core compatibility argument rests on a false "already shipped" premise. designs/http-adapter-pipeline.md:14-15 states "Phase 1 of the endo http controller/client pair (cli-http-client) shipped its defenses ... as a fixed pipeline with a flat set of policy knobs on the controller," and the document repeatedly leans on that premise to justify its compat choices: :1061-1062 ("rather than renaming the shipped set-bytes ... which would break a landed surface"), :1078 ("the already-approved call shape is preserved -- no silent breaking change to a shipped verb"). I verified against this worktree's actual tree: packages/cli/src/commands/ has no http.js and no http/ subtree at all, and grep -rn "set-rate\|set-bytes" --include=*.js . (excluding node_modules) returns zero hits anywhere in the repo. The only landed HTTP-client code is packages/exo-http-client/src/http-client.js, whose actual interface is HttpClientInterface.fetch(url, FetchOptionsShape) / HttpClientControlInterface.{inspect, setMaxRequestsPerMinute, setMaxResponseBytes, ...} — a fetch-first shape, not the request(RequestShape, context) onion-stage interface this design assumes throughout (:234-262). designs/README.md's own status table lists cli-http-client as Proposed, not Implemented, which corroborates that the CLI verb tree this document claims to be preserving does not exist yet. [proposed-rule: a design doc that characterizes a prior surface as "shipped"/"landed" must be checked against the actual source tree (grep the claimed identifiers) before that framing is used to justify a compatibility decision; cite the file:line of the landed code, not just a design-review PR link.]

    This matters for a migrator read specifically because the entire "no silent breaking change" argument (§ Exo and CLI surface additions) is built on preserving a caller that doesn't exist in code yet — PR #286 is a design-review approval, not a merge of endo http mk. If this document is handed to a builder as-is, the builder may believe Phase 1's CLI is a real baseline to diff against, when in fact Phase 1 itself (the cli-http-client design) and this elaboration will need to be built together, and the actual pre-existing surface to reconcile against is exo-http-client's fetch/inspect/setMaxRequestsPerMinute/setMaxResponseBytes, not the request/set-rate/set-bytes names used here. Recommend: change "shipped"/"landed" to "approved" or "designed" throughout, and add a short section reconciling the proposed request-based stage interface with the actually-landed fetch-based HttpClientInterface/HttpClientControlInterface in packages/exo-http-client.

Notes (out of scope but worth flagging):

  • No changeset is expected or missing here — this PR touches only designs/ (design-doc-only PR, Status: Proposed), consistent with every other design(...) commit precedent in this repo's history. [rule: skills/changeset-discipline/SKILL.md]

Self-improvement: none this round — the miss this finding closes (asserting "shipped" without grepping the tree) is now captured as a proposed rule above rather than a private habit; no other adjustment to roles/jurors/migrator/AGENT.md or its skills is warranted from this pass.

locksmith

locksmith — PR #992, kriscendobot/endo-but-for-bots

Scope note: git diff llm...HEAD returns 3422 files (493 commits) because the llm base ref in this worktree is stale relative to this branch's actual fork point — the vast majority of the diff is inherited history (unrelated feature branches merged along the way: @endo/reminder, jsonl-transcript, mount-glob/grep, cbor adoption, etc.), not this PR's content. The PR's actual substance is the six design(http): ... commits at HEAD, which add designs/http-adapter-pipeline.md (1408 lines, new) and a 6-line pointer from designs/cli-http-client.md. I reviewed that substance; the inherited history is out of the locksmith's remit here (already-landed, separately-reviewed work).

locksmith

Verdict: approve

Findings:

  • designs/http-adapter-pipeline.md:693-697 claims a compromised meter stage "can therefore drain at most the charge account's limit... and only until expiresAt," but the ChargeAccountInterface sketch at designs/http-adapter-pipeline.md:726-758 shows only a balance-bounded reserve(operationId, costMax) and getBalance() — no method or docblock note ties expiresAt/singleUse (both named as makeChargeAccount constructor args at line 689, owned by ertp-credits.md) to reserve's admission check. Since M.interface guards call/return shape, not behavior, this is silent-by-omission rather than wrong, but a builder implementing straight from this sketch could ship a reserve() that never checks expiry, which would falsify the "only until expiresAt" attenuation claim this design leans on. Add a one-line note in the interface comment that reserve MUST additionally reject on an expired/exhausted account, deferring the mechanism to ertp-credits.md. [proposed-rule: a design doc's interface sketch for an attenuator must either show every narrowing check the surrounding prose claims for it, or explicitly flag which checks are deferred to a cited sibling design]

Notes (out of scope but worth flagging):

  • The design is otherwise unusually rigorous on capability flow for a docs-only PR — worth naming since it's the seat's recurring finding elsewhere: CallerContextShape structurally excludes origin/requestId/attempt/any capability (line 205-225); StageContext carries no capability, the live meter reservation stays in the meter's own closure (line 216-221); attachChargeAccount is gated as "structural, not a provenance test" (line 1093-1104) rather than an if (readOnly)-style runtime check; and the test plan pins both of these with listed static-assertion tests (line 1218-1235). No runtime-flag attenuation or unhardened-surface pattern found — the two recurring locksmith findings this seat watches for. [rule: roles/jurors/locksmith/AGENT.md]

Self-improvement: none — the brief's two recurring-finding categories (runtime-flag attenuation, capability grants in docs-only PRs) were both actively and correctly guarded against in this design; no gap in the brief itself surfaced.

warden

warden — PR #992 (kriscendobot/endo-but-for-bots)

Scope note: This PR's diff (llm...HEAD) touches 3,422 files, including a large new Rust ironhorse engine tree outside the JS/SES surface. I sampled the SES-relevant JS surface: the new @endo/cbor, @endo/ascii, @endo/sha256 packages, @endo/reminder, and @endo/thixotrope, plus confirmed packages/ses/src/{enablements,permits}.js diffs are prose-only (no data changes to the repair/permit tables). I did not exhaustively review all 3,422 files.

Verdict: approve

Findings:
(none must-fix or should-fix)

Notes (out of scope but worth flagging):

  • packages/cbor/index.js:33-41: CANONICAL_NAN is deliberately left unhardened with an explicit rationale (typed arrays can't be frozen; the binding never escapes the module) — worth calling out as a model example of correct harden-discipline reasoning for future juror calibration, not a defect. [rule: AGENTS.md § Hardened JavaScript (SES) conventions]
  • packages/thixotrope/src/worker-peer-xs.js:28-93: sets/reads several globalThis properties (thixotropeSend, thixotropeTrace, console, thixotropeDispatch). This is the XS native-bridge boot script (documented as running "after the lockdown boot script"), analogous to existing daemon worker-bootstrap patterns elsewhere in the repo — not user/guest-reachable surface, so not a finding, but flagged in case a future round wants an explicit standing rule for XS-bootstrap globalThis use. [proposed-rule: XS/native-bridge bootstrap modules may assign globalThis properties to establish the host duct before lockdown-protected code runs, provided the assignment happens once at boot and is never reachable from guest/user code.]
  • packages/thixotrope/src/daemon.js and worker-peer.js consistently harden() every object crossing an E()/OCapN boundary, including copying caller-supplied endowment records before hardening them ("Implicit harden" comment at daemon.js:699) — correct discipline, no action needed.

Self-improvement: no new pitfall or reusable technique surfaced this round; the cbor package's non-harden-with-rationale pattern (packages/cbor/index.js:33) is a good addition candidate for roles/jurors/warden/AGENT.md § Operating norms as a worked example of a justified harden omission, distinct from the "unguarded global" failure mode the role already calls out — deferring since this is a single observation, below the ≥3-engagement threshold for a new rule.

saboteur

saboteur

Verdict: request-changes

Findings:

  • contentLength gives no ceiling against maxRequestBytes for the declared case — only the omitted-declaration path is capped (designs/http-adapter-pipeline.md:544-563). A guest that always declares an inflated contentLength (and, in a metered deployment, can pay for it — or, in an unmetered/pre-3.5 deployment, pays nothing at all) ships a request body of unbounded size, exactly the outbound flood maxRequestBytes was introduced to bound. This directly contradicts § SSRF/DoS posture's claim that "the byte-cap is strengthened into double duty" and "a guest's authority only shrinks" (designs/http-adapter-pipeline.md:1152-1159): for the declared-length path, authority does not shrink, it becomes unbounded modulo willingness to pay. The fix is to enforce min(declared contentLength, maxRequestBytes) as the transport ceiling in both branches, the same way maxResponseBytes truncates regardless of how the response declares its own Content-Length. Must-fix: it falsifies a claim the design itself makes. [proposed-rule: a dual-mode size cap (declared value vs. fallback default) must enforce the policy ceiling on both branches, not only the fallback branch]
  • Half-open probe admission has no stated atomicity guarantee, unlike the meter's explicit "no intervening await" MUST for reserve (designs/http-adapter-pipeline.md:740-748 vs. 965-976). A burst of concurrent attempts arriving while the breaker is half-open can each observe "a probe slot is available" before any of them is recorded as consumed, admitting more than halfOpenProbes concurrent probes — the same TOCTOU class the meter section was written specifically to close. halfOpenProbes: 2 also leaves the "majority" tie (1-1) undefined. Should-fix: state the same atomic check-and-admit discipline for probe admission, and a tie-break rule for even halfOpenProbes. [proposed-rule: every admission-counting mechanism in one design must carry the same atomicity guarantee as its sibling mechanisms, not just the one called out first]
  • Controller-only numeric config (maxAttempts, failureThreshold, halfOpenProbes, windowMs, cooldownMs in setRetry/setBreaker, designs/http-adapter-pipeline.md:880,967) gets none of the FiniteNonNegative/IntegerNonNegative rigor the guest-facing quantities get (designs/http-adapter-pipeline.md:170-192). halfOpenProbes: 0 or maxAttempts: 0 are unaddressed boundary values with undefined breaker/retry behavior. Should-fix, lower severity since it's host-set, not guest-adversarial. [proposed-rule: a numeric policy field on any admission/config verb needs the same finite/non-negative/integer boundary guard as a caller-facing field of the same shape]

Notes (out of scope but worth flagging):

  • This overlaps the breaker's invariant remit (§ SSRF/DoS posture's "none remove a gate" claim); flagged briefly here per the saboteur's narrow adversarial-input-falsifies-invariant overlap, full invariant audit left to the breaker seat. [rule: roles/jurors/saboteur/AGENT.md § Secondary surface]

breaker

breaker

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:902-912 (§ 4 Retryable classes) enumerates retry-eligible and retry-excluded outcomes exhaustively for every failure type it names — connection errors, timeouts, 5xx/429 retry; 4xx-other-than-429, origin/method/header rejection, and InsufficientFunds do not — but it never classifies RateLimitError. The design elsewhere goes out of its way to pin the disposition of the other two internal-stage rejections (CircuitOpenError is explicitly "terminal" at line 380-381 and again at 1007-1008; InsufficientFunds is explicitly excluded at line 906-907), so the omission of RateLimitError from either list is a gap, not an implied "obviously terminal." Since retry's next is the breaker whose next is the rate stage (§ Canonical stage order), retry directly observes a RateLimitError rejection bubbling up from the rate stage on every attempt. An implementer following this design as written could retry a rate-exhausted attempt immediately (no Retry-After-style floor is defined for it, unlike the HTTP-429 case at line 903-904), producing a tight maxAttempts-bounded retry burst against an already-exhausted per-client or per-controller window — exactly the "retry storm" the position rationale at line 855-857 (rate sits below retry "so a retry storm cannot bypass the window") assumes won't happen, but only prevents the storm from spending funds, not from hammering the window itself with no backoff. Add RateLimitError to § 4's retryable/non-retryable enumeration and state whether the internal retryAfterMs seeds the backoff floor the way the HTTP Retry-After does. [proposed-rule: a design's retry-classification section must be a closed, exhaustive table over every structured-error name the pipeline itself defines, not just the ones inherited from the transport.]

  • designs/http-adapter-pipeline.md:740-748 states the charge-account reserve atomicity as a "load-bearing MUST" specifically because concurrent guests or a retry storm could otherwise exploit a check-then-draw window. The circuit breaker's half-open admission (line 969-970, "admits exactly halfOpenProbes probe requests") is the same shape of shared, per-origin mutable counter under concurrent guest calls, but the design never states the equivalent atomicity guarantee for it. Two co-guests' requests landing in the same half-open window could, absent an explicit no-intervening-await requirement on the probe-admission check-and-increment, both be admitted as "probe 1," letting more than halfOpenProbes requests reach a still-unproven origin. Likely safe in practice under the same single-vat sequential-processing assumption that justifies the charge-account MUST, but the document should say so explicitly rather than leaving readers to infer it only for the account. [proposed-rule: any per-origin/per-controller shared-state admission counter reachable by concurrent guests states its atomicity guarantee explicitly, not only for the charge account.]

  • designs/http-adapter-pipeline.md:687-697 ("Who holds what") never states a one-charge-account-per-controller invariant. The blast-radius claim — "a compromised meter stage can therefore drain at most the charge account's limit" — silently assumes the account is not shared across controllers; nothing in the design forbids an integration from endowing the same ChargeAccount to two controllers' meter stages, which would let a compromised meter in controller A drain funds reserved for controller B, undermining the "controller is the trust boundary" framing the breaker-scope section relies on at line 984-986. Comment-only since this is an integration-provisioning discipline, not a pipeline mechanism, but worth an explicit sentence. [proposed-rule: designs that scope a shared secret/capability to a trust boundary state the cardinality invariant (one-per-boundary) explicitly, not just the consequence that follows from it.]

Self-improvement: none — roles/jurors/breaker/AGENT.md and skills/adversarial-tests/SKILL.md covered a design-only diff adequately; no gap in the brief surfaced this round.

purist

Per-juror block — purist

PR: kriscendobot/endo-but-for-bots#992 (design-http-adapter-pipeline)
Scope note: git diff llm...HEAD in the assigned worktree spans 493 commits / 3422 files because the local llm ref is three weeks stale relative to this branch's fork point — not the actual PR content. I isolated the true content with git diff 43ff29d78^...HEAD, which is exactly designs/http-adapter-pipeline.md (new, 1408 lines) plus two small cross-reference edits to designs/README.md and designs/cli-http-client.md. This is a design doc (Proposed status), not implementation code, so findings below apply the purist lens to the doc's own interface sketches.

Verdict: comment-only

Findings

  1. [should-fix] MeterReservationInterface breaks the harden-the-interface family pattern. designs/http-adapter-pipeline.md:711-723 defines MeterReservationInterface via M.interface(...) with no following harden(MeterReservationInterface). Its three siblings in the same design — HttpClientInterface (:233, hardened :246), HttpStageInterface (:252, hardened :267), and ChargeAccountInterface (:726, hardened :758) — all harden the interface guard immediately after definition. An unhardened M.interface result is exactly the kind of "this new symbol violates the ocap-shape invariants the rest of the module assumes" gap the purist/warden overlap exists for: an interface guard used to police an exo facet's calls should itself be tamper-proof, and three-of-four hardened is a family-consistency lapse a builder implementing this design would likely propagate. [proposed-rule: every M.interface(...) binding introduced in a design or package must be immediately hardened, or the omission must be explained]

  2. [comment-only] IntegerNonNegative is not actually a distinct pattern. designs/http-adapter-pipeline.md:192: const IntegerNonNegative = FiniteNonNegative; // + Number.isInteger() at the boundary. The design is explicit that @endo/patterns has no integer combinator, so this is a deliberate, documented tradeoff — not a bug. But the Test-plan section (e.g. "Count fields reject fractional values ... rejected at the boundary (the IntegerNonNegative integrality assertion)") reads as if the shape rejects 0.5, when the shape is byte-identical to FiniteNonNegative and the actual integer check happens only in hand-written Exo-method-body assertions elsewhere. A reader skimming the M.interface tables (e.g. settle: M.call(M.string(), MeasureShape)... at :711, where MeasureShape fields are typed IntegerNonNegative) would reasonably but wrongly conclude the guard itself throws on 0.5. Worth a one-line callout at the IntegerNonNegative definition (not just the Test-plan prose) that the name is a documentation label, not an enforced Pattern distinction, given the family it's presented alongside (FiniteNonNegative) is self-enforcing.

  3. [comment-only] Family-consistency of the name-discriminant error shape is scoped only to the three new errors. designs/http-adapter-pipeline.md § 2 (~:866-880) gives InsufficientFundsError/RateLimitError/CircuitOpenError a shared name discriminant "so all three get the same shape treatment rather than one being spelled and the others left as free-floating names" — but the pre-existing Phase 1 pre-flight rejections (origin/method/header, "the Phase 1 structured error" referenced in § Canonical stage order) are never confirmed to carry the same name discriminant. A caller that error.name-matches across the whole pipeline (pre-flight and onion) needs that guarantee stated, not just implied by the "all three" phrasing. [rule: roles/jurors/purist family-consistency axis — "what about the rest of the family" question]

Self-improvement: None — the design doc's authors (already 5 panel rounds deep) had clearly already internalized the purist lens (NaN/Infinity rejection, boundary-vs-caller measurement, structural non-reachability of the charge account); the remaining gaps were narrow and mechanical (one un-hardened interface, one same-object type alias). No new rule to propose beyond the one above.

spec-keeper

spec-keeper — PR #992 (kriscendobot/endo-but-for-bots)

Note on scope: git diff llm...HEAD in this worktree returns 3422 files (llm is a stale local ref, ~487 commits behind this branch's own base). The actual PR content is the design-branch commits (43ff29d78..33d202fdf), which touch only designs/http-adapter-pipeline.md (new, 1408 lines), designs/cli-http-client.md, and designs/README.md. Reviewed that scope.

Verdict: comment-only

Findings

1. [should-fix] IntegerNonNegative's integrality is not actually enforced where the Test Plan claims it is.
designs/http-adapter-pipeline.md (§ The stage interface) defines IntegerNonNegative = FiniteNonNegative; // + Number.isInteger() at the boundary — i.e. the pattern itself admits 0.5; integrality is deferred to a manual runtime assertion the doc says lives at "the meter's boundary." But IntegerNonNegative is used directly as the M.interface/M.splitRecord field guard for contentLength (RequestShape), bytesRead/elapsedMs (MeasureShape, the settle() argument), and maxRequestBytes/maxResponseBytes — entry points other than "the meter's boundary" (e.g. the settle() call on MeterReservationInterface, or the setMaxRequestBytes(n) controller verb). The Test Plan (§ Test plan, "Count fields reject fractional values") then asserts "a contentLength ... of 0.5 is rejected at the boundary (the IntegerNonNegative integrality assertion)" as if the alias itself performs the check — contradicting its own definition. A future implementer following only the interface-guard code will find M.interface silently admits 0.5 everywhere except wherever someone remembered to add the manual Number.isInteger throw. [proposed-rule: designs/*.md pattern-guard aliases whose name implies a constraint stronger than their own combinator definition (e.g. Integer* aliased to a non-integer-checking pattern) must either name every enforcement site or state plainly that the M.interface guard alone does not carry the named constraint.]

2. [comment-only] Cost-formula pseudocode doesn't show the omitted-contentLength substitution.
§ "The cost function" writes price.perByteRequest * BigInt(request.contentLength) unconditionally, while prose two sections later says an omitted contentLength bills maxRequestBytes instead. A one-line contentLength ?? maxRequestBytes in the formula (or an explicit note) would save an implementer from re-deriving it from prose.

3. [comment-only] Hardening inconsistency.
ChargeAccountInterface/HttpClientInterface/HttpStageInterface/MeterReservationInterface are each followed by harden(...); FiniteNonNegative, MeasureShape, StageContextShape, CallerContextShape are not. Low stakes in a design doc, but since the design elsewhere treats hardening as load-bearing (immutable StageContext, captured-at-construction discipline), applying it uniformly to every exported const would match the doc's own stated discipline.

No must-fix items: the NaN/Infinity/BigInt-mixing reasoning (rank ordering, BigInt(0.5)RangeError, 5n * 3TypeError) is spec-accurate, and the reserve/settle numeric invariants hold by construction.

Self-improvement: none this pass — the review scope note above (stale llm ref) is worth escalating to whoever maintains this worktree, but that's a worktree-hygiene issue, not a lens gap.

wire-watcher

wire-watcher

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:711-723MeterReservationInterface.settle/release state the idempotency guarantee in only one direction: "a release after a prior settle or release is a no-op" (line 720), but nothing states what settle() does if release() already ran first. This race is plausible, not hypothetical: the shared deadline/cancellation can fire mid-flight (§4, "the deadline stops a doomed attempt") while a late transport completion still reports bytesRead/elapsedMs and calls settle() on the same reservation whose byte/time terms release() already returned to available. If settle() naively moves costActual out of reserved, funds already refunded to available get double-counted — a real accounting/trust-boundary bug at exactly the reserve/settle state machine this design otherwise specifies carefully (the >= admission rule, the atomic no-await draw). The Test Plan (§ "Metering reserve/settle" and the idempotency items, lines 1255-1281) never exercises settle-after-release. Spell the terminal-state guard symmetrically (first of settle/release wins; the other is a no-op returning the same receipt) and add the missing test. [proposed-rule: a ledger reservation's terminal transitions (settle, release) must be specified and tested as mutually exclusive and idempotent across both methods, not just idempotent within each method]

  • designs/http-adapter-pipeline.md:612-628,636-638,284-293operationId (`${requestId}:${attempt}`) and measurementId (`${operationId}:measure`) are composed by plain string concatenation over a requestId whose format is described only as "the daemon's unforgeable-id source," never pinned. If that generator's output can contain :, two distinct (requestId, attempt) pairs could compose to the same idempotency key, defeating the no-double-reserve property §1 rests on. Pin requestId's character set with a regex assertion (or use a non-ambiguous separator/structured tuple key) so the composition is collision-safe by construction, not by assumption about an unspecified generator. [proposed-rule: a composite idempotency key built by string-concatenating component fields must pin each component's character set so the composition cannot collide across distinct component tuples]

  • designs/http-adapter-pipeline.md:965-976,1002-1013 — The breaker sits inside retry and is invoked once per attempt (line 1003), and half-open admits exactly halfOpenProbes requests system-wide with majority-success closing it. Because retry itself can generate multiple attempts against the same logical call, a single guest's own retry loop can consume some or all of the half-open probe budget, so the "majority-success" quorum meant to sample origin health can instead be decided by one caller's repeated attempts rather than independent evidence. Worth a note on whether half-open admission should also consider spreading probes across distinct callers/requestIds. [proposed-rule: a circuit breaker's half-open quorum must ensure probe diversity across distinct requesters when a single caller's retry loop could exhaust the entire probe budget]

Notes (out of scope but worth flagging):

  • designs/http-adapter-pipeline.md:223StageContextShape's attempt: M.number() is a bare numeric guard, the exact pattern the document's own opening comment (lines 160-175) warns fails open on NaN/Infinity. Not attacker-reachable today (CallerContextShape excludes attempt), but inconsistent with the document's stated numeric discipline; tighten to IntegerNonNegative for defense-in-depth. [rule: AGENTS.md § Numeric domain]

Self-improvement: none — the review procedure held; the one gotcha (this PR's llm...HEAD diff drags in ~490 unrelated commits from a stale base pointer) is worth a note to the panel/gardener about isolating a design-only PR's own commits via git diff <parent-of-first-PR-commit>...<HEAD> when the base ref is stale, but that's a one-off dispatch-worktree quirk, not yet a pattern across ≥3 engagements.

engine-realist

engine-realist review — PR #992 (kriscendobot/endo-but-for-bots)

Scope note: the llm...HEAD diff is ~3,400 files because this worktree's local llm ref is stale relative to the branch's actual merge base (upstream/llm-9d86783, tag-visible in the log). The PR's real content is a single design doc: designs/http-adapter-pipeline.md (confirmed via git diff 43ff29d78^..HEAD --stat). Reviewed that.

engine-realist

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:740-748 states the ChargeAccountInterface.reserve implementation "MUST perform the balance check and the draw with NO intervening await," explicitly to close a TOCTOU window when "the account may be a thin adapter over a ledger in another vat." But :761-762 (and the dependency-table row at :112) describes the charge account as exactly that: "a thin adapter over the gateway's ResourceLedger (getBalance, chargeBalance, ...)" — two separate methods, not one atomic reserve primitive. Nothing in this doc or in designs/gateway-package.md (grep for chargeBalance/atomic turns up only the method-name mention at gateway-package.md:411-412,1224, no atomicity contract) pins down that chargeBalance itself performs check-and-deduct in one synchronous method body at the vat that owns the balance. If a future implementer composes the adapter as getBalance() (await) then chargeBalance() (await) — the natural reading of "thin adapter over getBalance/chargeBalance" — the exact cross-vat overdraw race this section warns against reopens, and two concurrent metered requests (or a retry storm, which this pipeline explicitly retries per-attempt) could jointly drain more than limit. Close the gap before Phase 3.5 implementation: either state here that chargeBalance alone (no getBalance in the reserve path) must be the atomic primitive, or add that requirement to gateway-package.md's ResourceLedger contract so the atomicity guarantee doesn't rest solely on an unenforced MUST in a downstream consumer doc. [proposed-rule: a design that requires cross-vat atomic check-and-draw must name which single remote method call carries that atomicity, not merely assert the property on the composed adapter.]

Notes (out of scope but worth flagging):

  • The rest of the engine/vat-lifecycle surface is handled well for a design doc: numeric-domain bigint/number split matches AGENTS.md, breaker state is correctly kept ephemeral with the restart-as-half-open rationale spelled out (§ Open questions), no gratuitous WeakMap/WeakRef use, and requestId minting explicitly avoids a per-restart counter to prevent post-restart id collision. No action needed. [rule: roles/jurors/engine-realist/AGENT.md § Storage choice, § Allocation and GC budget]

Self-improvement: none — first pass on this design; the atomicity-gap pattern (an adapter's "MUST be atomic" clause resting on an unspecified downstream primitive) is worth watching for on future gateway/ledger-adjacent designs.

integrator

integrator

Verdict: request-changes

Findings:

  • designs/http-adapter-pipeline.md:1125,1132 introduces "Phase 3.5, Metering + Fees" and "Phase 3.6, Rate aggregate + Retries" as new labels in the endo http client's own phase sequence. But designs/daemon-agent-tools.md:477,492 already owns Phase 3.5 ("Local mount-bridged git tools — landed") and Phase 3.6 ("Network (HTTP) tier" — the makeHttpTool binding over this exact confined-fetch client, also cited from designs/endo-fetch.md:170). Both Phase-3.6s sit in the same conceptual neighborhood (HTTP tooling over the confined client), so a reader who has just learned "daemon-agent-tools Phase 3.6 = HTTP tool binding" will hit this doc's unrelated "Phase 3.6 = rate-aggregate + retries" and reasonably conflate them. The project has already paid for exactly this failure mode once — designs/daemon-git-next-steps.md:85,136 calls out "Phase-numbering drift between code and spec" as a named hazard and adds an explicit "Numbering caveat for builders" to disentangle two clashing Phase-3 spaces. This PR's § Staging into the Phase plan (designs/http-adapter-pipeline.md:1106) argues cli-http-client "does not yet enumerate a Phase 3/4 breakdown of its own," which is true, but doesn't check whether the numerals it borrows are already spoken for elsewhere in the same feature area — they are. Recommend either a distinguishing prefix (e.g. "Pipeline Phase 3.5/3.6") or an explicit disambiguation caveat matching the one daemon-git-next-steps.md already uses for the sibling clash. [rule: roles/jurors/integrator/AGENT.md § Concept-namespace coherence]

Notes (out of scope but worth flagging):

  • The doc centers on a 5-layer "onion" (retry ⊃ breaker ⊃ rate ⊃ meter ⊃ transport, designs/http-adapter-pipeline.md:349-407) but renders the order only as prose + a table, no diagram. 27 other design docs in this repo use mermaid flowchart/sequenceDiagram for exactly this kind of ordered-composition content; an ordered-wrapping structure is a strong mermaid candidate and would read faster than the table for a future implementer. Not blocking — the table is precise and complete. [rule: roles/jurors/integrator/AGENT.md § Diagram maintainability / Convention probe]
  • Everything else checked clean: all six Relationship/Dependencies table links resolve to real files (cli-http-client, http-confine, endo-fetch, daemon-xs-worker-metering, gateway-package, trust-on-first-bind); the designs/README.md summary-table arithmetic (44+27+40+27+2+7+2+1+2 = 152) and Proposed/total deltas (26→27, 151→152) both check out; the Test plan pins every claim with a concrete assertion rather than narrative (designs/http-adapter-pipeline.md:1209-1337); the six-commit round-by-round fix history matches this project's expected panel fixer-loop shape.

Self-improvement: nothing this time.

benchmarker

Per-juror block — benchmarker — PR #992 (kriscendobot/endo-but-for-bots)

Verdict: approve

Scope note: The diff (llm...HEAD, resolved via the actual PR content 43ff29d78~1..33d202fdf since the worktree's local llm ref is stale) is design-doc-only: a new designs/http-adapter-pipeline.md (1408 lines) plus small designs/README.md / designs/cli-http-client.md bookkeeping edits. The document's own Scope section states "Out of scope: Production implementation beyond illustrative interface sketches (this is a design task)" — no code lands in this PR.

Findings: none.

Walked every quantitative/cost claim in the document and the five round-1..round-5 commit messages for an open optimization thread:

  • The numeric-representation choices (bigint for monetary/reservation quantities, number/IntegerNonNegative for bounded per-request counts, the ceil rounding rule) are justified purely on domain-correctness grounds — "per this repo's AGENTS.md § Numeric domain they are bigint" and the costActual <= costMax invariant — not on speed. AGENTS.md § Numeric domain requires a benchmark only for a representation choice "made for speed"; this one explicitly is not, so no closure is owed here. [rule: AGENTS.md § Numeric domain]
  • The qualitative "cheap"/"cost" language around exo-stage hops ("costs one eventual-send per hop (cheap same-vat) and buys cross-vat composition for free" in Alt A; "a rate refusal is cheap and never touches the purse" § 3) is CapTP-model architectural rationale for a design decision, not a proposed optimization asserted as a settled, unverified performance property. Nothing here claims a benchmarked or benchmarkable speedup.
  • The one place a genuine speed/cost tradeoff appears — "Reservation granularity for streaming bodies" (finer-but-more-round-trips vs. coarser-and-cheaper settlement) — is explicitly left in Open questions with a proposed default and rationale, not asserted as closed. That is the "not pursuing/deferred, with a one-line reason" shape my role accepts, applied to an open item rather than a stalled claim. [rule: roles/jurors/benchmarker/AGENT.md § Primary surface]
  • No BENCH.md or benchmark report is added or touched by this diff, so there is nothing to check against skills/benchmark-comparative-report/SKILL.md.

No PR review-thread comments were reachable (PR #992 does not resolve via gh api repos/kriscendobot/endo-but-for-bots/pulls/992, consistent with this being an in-flight gauntlet dispatch); the commit-message trail (5 must-fix rounds already addressing critic/skeptic/decomplector/ergonomist/novice/copyeditor/pedant/breaker/corner-prober/assessor/spec-keeper/stylist/purist/migrator/integrator/archivist/saboteur findings) surfaces no benchmarker-relevant gap.

Self-improvement: none — the seat's cite-or-propose discipline and design-only-PR exemption (no code, no BENCH.md) matched this PR cleanly with the existing brief; no rule gap found.

changeset-auditor

Now I have everything needed to write the final block.

changeset-auditor

Verdict: request-changes

Findings:

  • .changeset/add-endo-ascii.md, add-endo-fetch.md, add-endo-reminder.md, add-endo-jsonl-transcript.md, add-endo-ocapn-iroh.md each bump a brand-new package (@endo/ascii, @endo/fetch, @endo/reminder, @endo/jsonl-transcript, @endo/ocapn-iroh — none exist on llm) as minor. A first release must bump major (0.1.0 → 1.0.0); as written these would publish 0.2.0 instead. [rule: skills/changeset-discipline/SKILL.md § New-package initial release]
  • packages/fetch/package.json and packages/reminder/package.json ship "version": "0.0.0" at creation instead of the required 0.1.0. [rule: skills/changeset-discipline/SKILL.md § New-package initial release]
  • packages/sha256/package.json ships "version": "1.0.0" at creation (its changeset correctly bumps major, but the checked-in starting version should be 0.1.0, letting the major bump land the published 1.0.0 — not pre-set it by hand). [rule: skills/changeset-discipline/SKILL.md § New-package initial release]
  • packages/ocapn-iroh/CHANGELOG.md is a hand-authored one-line header (# @endo/ocapn-iroh), not the bare Conventional-Commits stub the exemplar (@endo/cancel) uses. [rule: skills/changeset-discipline/SKILL.md § New-package initial release]
  • Sentence-per-line is broken across nearly every changeset body in this PR (add-endo-ascii.md, add-endo-jsonl-transcript.md, add-endo-fetch.md, add-endo-reminder.md, add-endo-sha256.md, add-endo-ocapn-iroh.md): bodies are line-wrapped prose paragraphs with sentence boundaries mid-line rather than one sentence per line. The skill doesn't currently codify this (only the role brief does), so tagging as proposed. [proposed-rule: codify "one sentence per line in changeset bodies" as a bullet under skills/changeset-discipline/SKILL.md § What goes inside, matching the role brief's existing expectation]

Notes (out of scope but worth flagging):

  • Package-set and body-identifier coherence were spot-checked only on the 8 new-package changesets (all clean beyond the above) and not exhaustively re-derived across all 49 changesets against the 3422-file diff; a full sweep is out of scope for one seat's pass at this PR's size. [rule: skills/changeset-discipline/SKILL.md]

Self-improvement: none — the sentence-per-line proposed-rule above is the only gap surfaced; forwarding it per the cite-or-propose discipline covers it.

surfacer

This confirms the exact fix pattern that reminder's new package should have followed but didn't. I have enough for the verdict now.

surfacer — PR #992 (kriscendobot/endo-but-for-bots)

Verdict: request-changes

Findings:

  1. [must-fix-loop] packages/reminder/package.json exports subpath "./src/types.js": "./src/types.d.ts" unconditionally, but no ./src/types.js file exists in the package at all (ls packages/reminder/src/ shows only types.d.ts). A consumer resolving @endo/reminder/src/types.js at runtime (or any tool that doesn't special-case .d.ts) gets a TypeScript-syntax file fed to a JS loader — it isn't valid ECMAScript and will throw. This PR lands the correct fix for exactly this shape in a sibling new package, packages/jsonl-transcript: its top-level export uses the conditional { "types": "./types.d.ts", "default": "./index.js" } split and ships a real empty types.js (export {}) with a comment explaining it exists "only to provide a JavaScript counterpart for types.d.ts... for tooling compatibility." reminder's internal JSDoc (src/index.js:100, src/store.js:37/40/53/67, src/backoff.js:23, src/scheduler.js:30/671) all do @import ... from './types.js', which works for type-checking via TS's .js.d.ts co-location resolution without needing a real file — but promoting that same specifier to a public exports subpath (as this PR does) is what makes the missing real file and the missing types/default condition split a break. Either drop the ./src/types.js subpath (keep it internal-only, matching packages/fetch's types.ts, which correctly has no export entry) or fix it to the jsonl-transcript shape: an actual src/types.js (export {}) plus a { "types": ..., "default": ... } condition. [proposed-rule: a package.json exports subpath naming a .js specifier must resolve — for at least one condition — to a real, loadable .js file; a bare "./x.js": "./x.d.ts" mapping with no default/runtime counterpart is a coherence break, not a type-only shortcut.]

  2. [follow-up] packages/capn-web/src/index.js re-exports recordRemap and replayRemap (also declared in package.json via the main entry) but neither appears in README.md's Quick Start, API, or Transports sections. Softer per brief (undocumented exports are sometimes deliberate), and every other exported identifier (makeCapnWebSession, makeLoopbackPair, makeWebSocketTransport, makeMessagePortTransport, makeHttpBatchTransport, processHttpBatchBody, handleHttpBatchRequest, patchStreamForHarden, E, Far, makeExo) is documented, so not blocking this PR. [rule: roles/jurors/surfacer/AGENT.md § README's claimed-public surface]

All other new packages checked for four-way coherence (ascii, cbor, fetch, is-well-formed-string, ocapn-iroh, sha256, thixotrope, claude-sandbox) — exports map, index.js/entry-thunk, published types, and README's claimed surface — agree.

Self-improvement: none this cycle — the brief's four-way-coherence checklist (exports / thunk / types / README) was sufficient as written to find finding 1; no gap surfaced.


(Aggregate truncated to fit GitHub's review-body limit; remaining seat blocks reached the same disposition. Full aggregate retained in the panel rundir.)

model claude-opus-4-8 · harness claude · garden 4e95097a

- assessor: fix the cost formula's deadline units — deadline is an absolute
  timestamp, so effectiveDeadlineMs subtracts now (min((deadline ?? now+timeoutMs)
  - now, timeoutMs)); correct the worked example and the § Open questions restatement.
- typist: RequestShape.contentLength is IntegerNonNegative (discrete count), not
  FiniteNonNegative, agreeing with § Numeric domain and the two other declarations.
- saboteur: enforce maxRequestBytes as the outbound ceiling on the DECLARED
  contentLength branch too (reject a declared contentLength > maxRequestBytes at
  pre-flight), not only the omitted branch; add the test.
- breaker/assessor: classify the internal RateLimitError (and CircuitOpenError) in
  § 4 as terminal/not-auto-retried, distinct from an origin HTTP-429 resend; add test.
- curator/purist/spec-keeper: harden(MeterReservationInterface) and rename its tag
  to 'EndoMeterReservation' to match the Endo* interface family.
- wire-watcher: spell settle/release as mutually-exclusive terminal transitions
  (first-wins) to close the deadline-vs-late-completion double-count race; add test.
- stylist: rename req -> request throughout the sketches and prose.
- packager: designs/README.md milestone cell — seven controller verbs (was five),
  five concerns incl. fees (was four).
- fast-checker/corner-prober: add a property-based costActual <= costMax test.
Comment thread designs/http-adapter-pipeline.md Outdated
Comment on lines +256 to +264
// The pure cost probe forwarded DOWN the onion: stages above the meter
// pass it straight through to `next`; the meter stage answers it from
// its private PriceSchedule and returns without reserving. This is how a
// thin-forwarder client reaches the interior price without a direct
// meter reference -- composition opacity holds, the guest never names a
// stage. A chain with no meter stage answers 0.
estimateCost: M.callWhen(RequestShape)
.optional(StageContextShape)
.returns(FiniteNonNegative),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awkward since it creates a cross-cutting concern and is a bit of a smell because it won’t be the last time a middleware needs to communicate through intermediate middleware. We need an alternative. In this case, the middleware chain may need to be constructed in such a way that a stage can call forward to the next middleware's specialised estimateCost for the request, such that it is a contract between two middleware constructors, not middleware in general. Please don’t repeat “middleware”. Let’s call these adapters.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 53dfdd7.

The smell. You're right that a pure-probe method on the general stage interface is a cross-cutting concern — retry/breaker/rate each carried an inert estimateCost pass-through purely so the two ends of the chain could talk, and it would recur the next time one adapter needs a specific downstream adapter. So estimateCost is now off HttpStageInterface (which stays request + help).

The alternative. Cost quoting is now a specialized adapter-pair contractCostQuoteInterface, a bilateral facet the composer wires at construction between exactly the two constructors that share it (the meter provides it; the client's thin forwarder consumes it), the same shape next already uses. Non-participant adapters never see it; a meterless chain answers 0 via a named null quote instead of an interface-wide fallthrough. A new ### Specialized adapter-pair contracts section states the general pattern (an adapter calling forward to the next participant's specialized method, skipping non-participants) so the multi-party case is the same wiring rule applied transitively — the general answer to "it won't be the last time."

Terminology. "middleware" is now reserved for the in-process prior art (Koa/Express et al.); our construct is called adapters/stages throughout.

model claude-opus-4-8 · harness claude · garden 87ccd70a

kriscendobot added a commit to kriscendobot/garden that referenced this pull request Aug 16, 2026
…t a uniform-interface method

Address kriskowal's review on PR endojs#992: the pure cost probe on the general
HttpStageInterface forced every adapter (retry, breaker, rate) to forward a
pass-through it has no stake in -- a cross-cutting smell that would recur for
the next inter-adapter conversation. Remove estimateCost from the uniform
interface; introduce CostQuoteInterface, a bilateral facet the composer wires
at construction between exactly the two constructors that share it (meter
provides, client forwarder consumes), the same shape 'next' already uses.
Non-participant adapters keep request + help only; a meterless chain answers 0
via a named null quote. Generalizes to any future adapter-pair contract
(transitive forward-threading for multi-party chains).

Also honor 'call these adapters, not middleware': reserve 'middleware' for the
in-process prior art, name our construct adapters/stages throughout.

Updated the design's Updated date and the designs/README.md summary row.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriscendobot added a commit that referenced this pull request Aug 17, 2026
Address the panel's round-2 must-fix and should-fix items on PR #1014:

- archivist (must-fix): drop the dangling "write-once log" cross-reference in
  designs/cli-http-client.md; state plainly that Phase-1 tofu-auto is a distinct,
  narrower mechanism from the trust-on-first-bind addendum.
- fast-checker (must-fix): add two property tests over normalizeHttpClientOrigin
  — idempotence over the accepted-origin space (the docstring's canonical-form
  claim) and refusal of any path/query/fragment/userinfo suffix (the false-
  confinement boundary). Adds @fast-check/ava as a cli devDependency.
- prover (should-fix): extract the --origin collector, the --policy-mode
  validator, and the opts->args mapping into pure http-mk-policy.js helpers
  (collectHttpOrigin / parsePolicyModeFlag / httpMkArgsFromOpts) and unit-test
  them, pinning the last-wins accumulation regression and the swapped-destructure
  regression the daemon-driven test cannot see.
- scribe (should-fix): add the metering/rate-limiting forward link to
  #992 in the Landed CLI surface section.
- changeset-auditor (should-fix): rewrap the changeset one sentence per line.
- packager (should-fix): fix the stale <url> metavar to <origin> in the design
  synopsis.
- typist (should-fix): retype args.allowedOrigins as {string[] | undefined} in
  http-mk-policy.js and commands/http-mk.js to match the undefined-tolerant
  runtime.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriscendobot added a commit to kriscendobot/garden that referenced this pull request Aug 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants