feat(cli): add endo http mk to mint a confined HTTP client - #1014
feat(cli): add endo http mk to mint a confined HTTP client#1014kriscendobot wants to merge 14 commits into
Conversation
kriscendobot
left a comment
There was a problem hiding this comment.
Panel verdict — round 1: must-fix
Posted as a --comment review: GitHub blocks --request-changes on a self-authored (bot) PR; the verdict is carried in the body per the "Must-fix before merge" heading below.
Code panel (27 seats) reviewed PR #1014 against origin/llm...HEAD. Aggregate disposition: must-fix (22 seats request-changes, 4 comment-only, 1 approve).
Must-fix before merge
The fix stage must apply the request-changes / must-fix items from the per-seat blocks below, push them as review-feedback follow-up commits, and re-enter the gauntlet. Full per-seat aggregate follows.
assessor
Reviewed: git diff origin/llm...HEAD in the panel worktree (5 files, +308/−3). Verified live: full test file passes 5/5, yarn lint:types clean, eslint clean (the one warning at packages/cli/src/endo.js:2 is pre-existing), and every factual claim in the design/changeset checks out against the daemon (redirect: 'manual' at packages/http-confine/src/http-confine.js:547, WeakMap-held control at packages/daemon/src/manager.js:1204, defaults 60 / 1 MiB, modes strict|tofu-auto).
assessor
Verdict: request-changes
Findings:
- Re-running
mkon an existing pet name silently rebinds it and strands the previous client's revocation authority.packages/cli/src/commands/http-mk.js:57. Verified against a live daemon:endo http mk two --origin http://127.0.0.1:8081thenendo http mk two --origin https://other.example.comboth printtwoand exit 0; the name now denotes a second client under a different policy. BecausegetHttpClientControl(clientCap)is the only path to the control facet and it is keyed by the client cap in a host-side WeakMap, once the name is rebound the host can never revoke the first client — which still works under its old allowlist for any guest already introduced to it. The verb's own JSDoc says the host "retains the paired control facet … mutating and revoking it are a later phase's verbs"; the happy path already falsifies "retains".provideHttpClientinherits this fromprovideShell, but this PR is what makes it user-reachable. Refuse an already-bound name (or gate on--force), or state the rebinding in themkdescription and changeset. [rule: designs/cli-http-client.md § Landed CLI surface] --max-requests-per-minute/--max-response-bytescoerce with bareNumber(val)(packages/cli/src/endo.js:829,834), soabc→NaN,''→0,1.5,1e100all pass the!== undefinedgate athttp-mk.js:47-48and fail only after a daemon round trip. Confirmed output is a raw CapTP stack dump namingpolicy.maxRequestsPerMinute, not the flag the user typed. Deferring policy validity to the daemon is deliberate and documented; rejecting a non-integer flag locally is not the same thing, andpackages/cli/src/number-parse.jsis the in-repo precedent. [proposed-rule: a numeric CLI flag validates its own lexical shape locally and reports by flag name; only semantic validity defers to the daemon]- comment-only: the
--origincollector atpackages/cli/src/endo.js:820-826mutates Commander's default array in place; the codebase's collector (collectDeniedSegment,packages/cli/src/denied-segments.js:29) is non-mutating and default-tolerant. Not observable today —programand the[]literal are fresh permain()call — so this is consistency only. [proposed-rule: repeatable-option collectors reuse the non-mutatingcollectDeniedSegmentshape]
Notes (out of scope but worth flagging):
bin/endo.cjs:4discardsmain()'s return value, somain's carefully computed exit codes are lost: bareendo httpprints help,mainreturns 1, the process exits 0. Pre-existing, but the new action-lesshttpparent is a fresh way to reach it. [proposed-rule: the CLI entrypoint propagatesmain()'s returned exit code toprocess.exitCode]packages/cli/test/http-mk-command.test.js:57-73spins up a real HTTP server and never sends a request to it; the normalizer only parses the origin string, so any well-formedhttp://127.0.0.1:<port>would do. The comment reads as though the test exercises the fetch path. [proposed-rule: test fixtures assert only what the test actually exercises]
Self-improvement: the seat's leverage here came from running the verb against a live daemon rather than reading the diff — the rebinding hazard is invisible in the diff and invisible to the test suite. Worth encoding: for a PR that adds a new mint/provide CLI verb, exercise the verb twice on the same name before writing the block.
typist
typist
Verdict: request-changes
Findings
-
must-fix —
packages/cli/test/http-mk-command.test.js:68:/** @type {import('net').AddressInfo} */ (server.address())is an inlineimport()type reference inside a JSDoc@typetag. Fix: a top-of-file/** @import { AddressInfo } from 'node:net' */and a bareAddressInfoin the cast (node:net, matching this file'snode:httpimport). [rule:roles/jurors/typist/AGENT.md§ Inlineimport()type references in JSDoc tags; provenanceendojs/endo-but-for-bots#75reviewr3223741240]. The file's// @ts-nocheck(line 1) does not exempt it — the rule is textual and the probe is a gate the panel backstops. -
should-fix —
packages/cli/src/commands/http-mk.js:31:@param {string} [args.policyMode]widens a closed union. The daemon declaresHttpClientPolicyMode = 'strict' | 'tofu-auto'(packages/daemon/src/types.d.ts:417), and the doc text right on the same line already spells both members, so the type is strictly less informative than its own prose. Fix:@param {'strict' | 'tofu-auto'} [args.policyMode], or better, re-exportHttpClientPolicyModefrompackages/daemon/types.d.ts(it is absent from that entry'sexport type { ... }list, unlikeRetentionPath) and@importit, the pattern already used inpackages/cli/src/commands/{list,paths,trace}.js. [rule:roles/jurors/typist/AGENT.md§ Type-runtime drift is the recurring typist finding] -
should-fix —
packages/cli/src/commands/http-mk.js:47-55builds a deliberately partial policy record (comment: "the guard knobs are omitted... so the daemon applies its own defaults") and passes it toprovideHttpClient, whose declared parameter typeHttpClientPolicy(packages/daemon/src/types.d.ts:425-430, 1629-1632) has all four fields required.normalizeHttpClientPolicy(packages/daemon/src/host.js:222-275) defaults every one of them, so the signature over-constrains what the method actually accepts, and this PR is the first caller to depend on that. Nothing catches it becausepackages/cli/tsconfig.jsonsetscheckJs: false. Fix: give the parameter an input type (HttpClientPolicyInput={ allowedOrigins?: string[]; maxRequestsPerMinute?: number; maxResponseBytes?: number; policyMode?: HttpClientPolicyMode }), keepingHttpClientPolicyas the normalized, frozen formula record. [proposed-rule: when a callee normalizes-with-defaults, the parameter type is the input shape, not the normalized shape] -
comment-only —
packages/cli/src/endo.js:829,834:val => Number(val)yieldsNaNfor--max-requests-per-minute abc, so the@param {number}is honest only in the "NaN is a number" sense; the failure surfaces as the daemon's "must be a positive safe integer". Matches existing precedent atsrc/endo.js:515, so no change required here, but a validating parser would fail locally and keep the declared type meaningful.
No typist-hostile code points in the added lines; --origin accumulator, parsePetNamePath → string | string[] petName, and the agentNames typing all match the mktmp precedent.
Self-improvement: the checkJs: false on packages/cli means every JSDoc finding in this package is documentation-only and no gate will catch a regression — worth a standing note that CLI-package type findings need a seat, not a probe.
stylist
Reviewed the two-commit diff (origin/llm...HEAD): CLI endo http mk, its command module, a test file, a changeset, and a design-status edit.
stylist
Verdict: request-changes
Findings
1. must-fix. mk is the only spelling of a new public verb. packages/cli/src/endo.js:812 registers .command('mk <name>'); the module is packages/cli/src/commands/http-mk.js and the export is httpMk (:35). Everywhere else in this CLI, mk is an alias for a spelled-out verb (.command('make [file]').alias('mk'), packages/cli/src/endo.js:123-124) or half of a POSIX-echoing compound (mkdir, mktmp, mkhost, mkguest). A bare mk as the canonical, unaliased spelling is new to this package, and it is a plain abbreviation of make on a public surface. Fix: .command('make <name>').alias('mk'), module http-make.js, export httpMake; the alias preserves the ergonomics the design wanted. [rule: roles/jurors/stylist/AGENT.md § Abbreviated identifiers; skills/pre-push-gates/SKILL.md spell-out-identifiers]
2. must-fix. The design now contradicts itself about whether mk is a name. The PR sets Status to "Phase 1 (endo http mk) landed" (designs/cli-http-client.md:8, new section :25-70), but leaves standing :176 "(Verb names above are placeholder; final names land via the namer dispatch.)" and :234 "Identifier names are placeholders pending the namer dispatch." The document that supposedly licenses mk says mk is a placeholder awaiting a namer. Either run that dispatch or say explicitly in the new section that Phase 1 froze the placeholder and why. [rule: stylist secondary surface, name-versus-doc disagreement]
3. should-fix. addr at packages/cli/test/http-mk-command.test.js:68-69 should be address (or serverAddress). Freshly authored; it mirrors packages/daemon/test/ws-relay.test.js:65, but the never-abbreviate rule is mechanical and applies to new code. [rule: roles/jurors/stylist/AGENT.md § Abbreviated identifiers]
4. comment-only. (val, acc) at packages/cli/src/endo.js:820-822 and val => Number(val) at :829, :834 should read (value, accumulator) and value. Downgraded because it copies the four existing collectors at :231, :250, :282, :306 verbatim, and consistency with the file is a real defense; a sweep belongs in its own change.
5. comment-only (adjacent to remit). packages/cli/src/commands/http-mk.js:50 justifies the conditional spread with "its normalizer rejects an undefined numeric", but packages/daemon/src/host.js:242-243,254 defaults undefined guards (60, 1 MiB). The code shape is fine; the stated reason is false.
No redundant-word concatenations. --origin mapped to allowedOrigins at packages/cli/src/endo.js:842 is an accurate boundary rename.
Self-improvement: nothing this time.
packager
packager
Verdict: request-changes
Findings:
-
.changeset/cli-http-mk-phase-1.md:14-28— the last two paragraphs are implementation detail and process commentary, not release notes. "It rides the HTTP client that already landed on the daemon rather than introducing a new formula", "backed by@endo/exo-http-clientover@endo/http-confine", "reachable viagetHttpClientControl", "not the controller/client formula pair the original design assumed", "This is CLI-only; no@endo/daemonchange is needed", "follow in later phases", and the "(Phase 1 ofdesigns/cli-http-client.md)" framing all address a reviewer, not a downstream@endo/cliconsumer deciding whether to upgrade. That audience needs the verb, its flags, and what it mints — the first two paragraphs plus the synopsis block already deliver exactly that. This material is already in commit226b56a8bc's message nearly verbatim, which is its correct home. should-fix. [rule: skills/changeset-discipline/SKILL.md § What goes inside — "Omit implementation details" / "No process commentary"] -
designs/README.md:405— the row flipsProposed→Phase 1 landed (...), a status string with no canonical bucket word. Every other freeform entry in that column keeps a bucket as the leading token (Proposed (research note),Proposed (partially satisfied by \packages/genie`),Reference (research note)). The Totals line atdesigns/README.md:451is tallied by reading that column ("recounts the table by tallying the status column of the summary table") and still reads "26 Proposed" while this row is now uncountable. Fix: lead withIn Progressand demote the rest to the parenthetical, then shift Totals to Proposed 25 / In Progress 28. **should-fix.** [proposed-rule: a status flip indesigns/README.md`'s summary table keeps a canonical bucket word (Complete/Implemented, In Progress, Not Started, Proposed, Active, Reference, Deprecated, Draft, Superseded) as the leading token and syncs the Totals line in the same commit.] -
packages/cli/test/http-mk-command.test.js:52-71— the livehttp.createServer+listen+t.teardownis dead scaffolding: nothing ever dials it.normalizeHttpClientPolicy(packages/daemon/src/host.js:174-195) onlynew URL()-parses the entry and comparesparsed.origin; it never connects. The inline comment claiming the server makes the origin "real … the daemon's policy normalizer accepts" is inaccurate — a literalhttp://127.0.0.1:8080satisfies it identically. Drops ~20 lines and thenode:httpimport. comment-only. [rule: roles/jurors/packager/AGENT.md § Operating norms — does the diff carry only what the PR claims]
Notes (out of scope but worth flagging):
- Commit
32e4d80981removes a/* global process */line from a file commit226b56a8bcintroduced two commits earlier in the same PR, so226b56a8bcfailsyarn lintin isolation. This is the inverse of the usual packager complaint (the split is clean, not conflated), but a self-repair of this PR's own new file wants squashing into it. [rule: skills/changeset-discipline/SKILL.md § What goes inside — "fix-up commits responding to review feedback are part of that one change"] - Bump level is right:
'@endo/cli': minorfor a purely additive verb, single changeset, single package touched, no@endo/daemonedit needed (provideHttpClient/getHttpClientControlalready exist atpackages/daemon/src/host.js:927,946and inHostInterface). New command module placement and thewithEndoAgent({ os, process })shape matchmkhost.jsexactly. No generated-file or lockfile churn. [rule: skills/yarn-lock-separate-commit/SKILL.md]
Self-improvement: the counted-summary-table desync in designs/README.md is a recurring shape for this repo (its own prose records prior drift of two and four designs). Worth a standing packager check whenever a diff touches that table: read the leading token of the status cell, then confirm the Totals line moved.
archivist
Reviewed the committed diff (origin/llm...HEAD, 2 commits, 6 files) plus the daemon code the new prose describes.
archivist
Verdict: request-changes
Findings:
packages/cli/src/commands/http-mk.js:49-50— the comment's stated reason is false: "the daemon applies its own defaults otherwise (its normalizer rejects an undefined numeric)".normalizeHttpClientPolicyexplicitly acceptsundefinedand substitutes defaults (packages/daemon/src/host.js:243-244→ 60,:254-255→ 1 MiB,:265→'strict'); it rejects only a non-positive/non-safe integer. A reader following this comment will believe passing{ maxResponseBytes: undefined }throws, when it behaves identically to omitting it. Drop the parenthetical or restate the real motive (a minimal, self-describing policy record). must-fix [rule: roles/jurors/archivist/AGENT.md § docstring-vs-code disagreement]packages/cli/test/http-mk-command.test.js:47-50(as committed) — "Spin up a tiny HTTP server for the allowlist origin so the origin is a real, well-formed http(s) origin the daemon's policy normalizer accepts" misdescribes the code.assertHttpClientOrigin(packages/daemon/src/host.js:175-196) does a purely structuralnew URL+parsed.origin === origincheck, andmkissues no request, so nothing needs to be listening. Either say why a live port is used (an unambiguously free port) or drop the server; as written the comment teaches a false requirement. should-fix [rule: roles/jurors/archivist/AGENT.md § comments still describe the code they sit next to]designs/cli-http-client.md:29,32,37,58,70and.changeset/cli-http-mk-phase-1.md— new prose packs multiple sentences per line, while the rest of this design document is one sentence per line.AGENTS.md§ Markdown style: "Start each sentence on a new line so that diffs are per-sentence." should-fix [rule: AGENTS.md § Markdown style]packages/cli/src/commands/http-mk.js:32anddesigns/cli-http-client.md:52document--as <agent>with no host-only caveat, butprovideHttpClientlives onHostInterfaceonly (packages/daemon/src/interfaces.js:369, inside the block opened at:262);GuestInterface(:160) has no such method, and the siblingprovideShellis documented "Host-only; not exposed to guests" (packages/daemon/src/types.d.ts:1615). Say "another host agent". should-fix [rule: roles/jurors/archivist/AGENT.md § is new behavior documented]
Notes (out of scope but worth flagging):
- The document's H1 still reads "Controller + Client Pair under
endo http" while its own new § states that pair "no longer exists"; and the blanket caveat covers "the placeholder tables below", butdesigns/cli-http-client.md:166is a code block showingendo http mk <name> <origins...>(positional origins, not--originflags). One clause would close both. comment-only [rule: roles/jurors/archivist/AGENT.md § design-document Status accuracy] - The worktree is dirty and does not match HEAD:
packages/cli/src/commands/http-mk.json disk dropsallowedOrigins,from the policy record under a// MUTATION: origins droppedmarker. That line is present in the committed diff, so it is not a PR defect; flagging it so no seat reviews the mutated file as if it were the PR.
Self-improvement: the strongest finding this round came from reading a comment's causal claim ("the normalizer rejects X") back against the named function rather than accepting it as plausible; when a comment justifies a construct by asserting downstream behavior, open the downstream function every time.
prover
Prover — PR #1014 (endojs/endo-but-for-bots), feat(cli): add endo http mk)
Verdict: request-changes
Findings
-
must-fix —
packages/cli/test/http-mk-command.test.js:60asserts only thatmkechoesmy-httpand thatendo listshows it; nothing observes the policy or the capability type. Mutation-proven, both green on all 5 tests:- dropping
allowedOriginsfrom the policy record (packages/cli/src/commands/http-mk.js:48) — the daemon's normalizer treatsallowedOrigins === undefinedas an empty allowlist (packages/daemon/src/host.js:227), so the mutant mints a client that can reach nothing and ships green; - replacing
E(agent).provideHttpClient(parsedName, policy)withE(agent).provideGuest(parsedName)— a guest under the pet name also passes.
The verb's entire contract (one client, under this origin policy) is unpinned. Cheap fix, verified live against a real daemon: the minted client exposes anallowedOriginsmethod, soendo eval 'E(c).allowedOrigins()' c:my-http→[ 'http://127.0.0.1:<port>' ]kills both mutants in one assertion. [rule:skills/regression-evidence/SKILL.md— break the target in the obvious way and confirm the test reddens]
- dropping
-
should-fix —
--max-requests-per-minute,--max-response-bytesand--policy-modehave zero coverage, yet the changeset claims "a malformed origin or guard surfaces as its structured error on the CLI invocation". The claim holds (I confirmed--origin http://x.example/path,--max-requests-per-minute abc,--policy-mode tofu-prompteach exit 1 with the daemon's structured message) but nothing pins it; deleting any knob from the record athttp-mk.js:50-53reddens no test. An invalid-value case per knob is load-bearing — it fails only if the field actually reaches the daemon. [rule:skills/coverage-driven-testing/SKILL.md] -
comment-only —
test/http-mk-command.test.js:49-51:t.notRegex(stdout, /controller-name/ | /client-name/)can never redden — that surface was never implemented on any branch. Assertions that no regression can flip are decoration. [rule:skills/regression-evidence/SKILL.md] -
comment-only —
test/http-mk-command.test.js:63-79starts a real listener purely to obtain a port; the normalizer never connects (a deadhttp://127.0.0.1:8123mints fine). It implies end-to-end coverage the phase does not have; a literal origin string would say the same thing honestly. -
comment-only — the
Networkhelp section (src/endo.js:1110) is unpinned:grouped-help.js:100renders ungrouped commands underOther Commands:, soendo --helpstill matches/\bhttp\b/if the section is deleted.
Self-improvement: mutation-testing the diff directly (revert-in-place, re-run) was decisive here where reading the assertions alone would have read as adequate coverage; I will keep leading with the mutation, not the read.
curator
Reviewed the worktree diff (origin/llm...HEAD, 2 commits: endo http mk + a lint follow-up).
Juror: curator
Verdict: comment-only
Surface delta. @endo/cli is private with "exports": {} — no module-level export surface changes. The public surface here is the CLI verb tree: a new endo http parent (packages/cli/src/endo.js:806) with one subcommand mk <name> and flags --as, --origin (repeatable), --max-requests-per-minute, --max-response-bytes, --policy-mode; a new internal module packages/cli/src/commands/http-mk.js exporting httpMk; and a new Network help group. Nothing removed, nothing renamed, no daemon-side signature touched — the verb is a pure front-end over the already-landed host method provideHttpClient(petName, policy) (packages/daemon/src/host.js:927, guard packages/daemon/src/interfaces.js:369, NameOrPathShape, satisfied by parsePetNamePath). Verified rendering: endo --help, endo http, endo http mk --help all resolve correctly, and Commander v5.1 camel-cases the four-word flags as the destructure assumes.
Bump: correct. .changeset/cli-http-mk-phase-1.md is '@endo/cli': minor for an additive verb — matching precedent (cli-mount-denied-segments.md, retention-paths-phase-1.md), and .changeset/config.json sets privatePackages.version: true, so the entry is meaningful despite private: true. [rule: skills/changeset-discipline/SKILL.md]
Findings
-
should-fix —
--originadvertises a wider shape than the daemon accepts.packages/cli/src/endo.js:818reads "Allowed origin URL (http: or https:); repeat for multiple", and the JSDoc atpackages/cli/src/commands/http-mk.js:28says the same. The authority,assertHttpClientOrigin(packages/daemon/src/host.js:174-195), requires the entry to equalparsed.originverbatim —scheme://host[:port], no path/query/fragment, default ports normalized away.endo http mk x --origin https://api.example.com/v1is a natural reading of "origin URL" and fails at the daemon. A validated flag's description is its user-facing contract; name the constraint in the flag text and the JSDoc. [proposed-rule: a CLI flag whose value is validated in another package must state the accepted shape in its own--helpdescription, not only in the validator's error] -
comment-only —
policyMode's admissible set is re-declared in the CLI.HTTP_CLIENT_POLICY_MODES(packages/daemon/src/host.js:162) is canonical;packages/cli/src/endo.js:838restates "strict (default) or tofu-auto" as prose. Adding a third mode requires a matching edit here that nothing enforces. Same drift shape as the cross-package option-type rule — the daemon is the canonical home; ifHttpClientPolicy['policyMode']is exported as a type/const, prefer deriving the help string from it. [rule: roles/jurors/curator/AGENT.md § cross-package option types live in one canonical package] -
comment-only — the numeric guard flags coerce with
Number(val)and can smuggleNaNinto the policy record.packages/cli/src/endo.js:829,834:--max-requests-per-minute abcyieldsNaN, which is!== undefined, sohttp-mk.js:50-52puts it in the policy; the user gets the daemon's "must be a positive safe integer" without the flag name. The declared@param {number}isn't enforced at parse. Rejecting at the CLI parse step would keep the signature honest. Related: the three guard flags are advertised in help but no test exercises any of them —packages/cli/test/http-mk-command.test.jscovers only--originand the empty-allowlist error.
Design/status bookkeeping (designs/README.md:408, designs/cli-http-client.md § Landed CLI surface) accurately marks the controller/client-pair mint signature as historical and states the landed surface supersedes the tables below it — no finding.
Self-improvement: the recurring curator catch on this PR was not a bump mismatch but a help-text contract narrower than the validator behind it; worth carrying forward as a standing check whenever a CLI flag forwards an opaque record to another package's normalizer.
migrator
migrator
Verdict: request-changes
Findings:
-
packages/cli/src/endo.js:812—--originis described as "Allowed origin URL (http: or https:)", but the callee accepts a far narrower grammar:assertHttpClientOrigin(packages/daemon/src/host.js:175-195) requiresnew URL(entry).origin === entryverbatim. So the two forms a user actually pastes both fail —https://example.com/(trailing slash → origin ishttps://example.com) andhttps://example.com:443(default port normalized away), as do path-bearing entries. The only test origin (packages/cli/test/http-mk-command.test.js:70) is the canonical form, so nothing covers the failure. Fix: state the exact grammar in the option description and in the changeset's usage block (scheme://host[:port], no trailing slash, no default port, no path), and/or canonicalize the loss-less cases in the option coercer while still refusing path-bearing input (never silently strip a path — that would widen authority). should-fix [proposed-rule: a CLI flag's description must state the exact grammar its callee validates whenever the callee's acceptance is narrower than the flag's noun.] -
packages/cli/src/commands/http-mk.js:49-50— the comment justifies the conditional spread with "its normalizer rejects an undefined numeric". Inverted:normalizeHttpClientPolicydefaults undefined (60/1 MiB,packages/daemon/src/host.js:241-262) and rejects only non-positive / non-safe integers. The behavior is right, the stated contract is false, and it is exactly what a phase-2allow/denyauthor will build on. should-fix [rule: skills/gricean-maxims/SKILL.md] -
Bump level is correct and no cascade entry is missing:
@endo/cli: minorfor a purely additive verb, and@endo/daemon'sprovideHttpClientalready carries its own pendingminorin.changeset/http-tool-and-daemon-http-client.md, soworkspace:^(packages/cli/package.json:35) resolves to a daemon that has the method. The uncovered skew is runtime, not dependency: a daemon process started before the upgrade survives it, andendo http mkthen fails as an opaque interface-guard miss. One sentence in the changeset — "requiresendo restartagainst a pre-existing daemon" — closes it. comment-only [rule: skills/changeset-discipline/SKILL.md]
Notes (out of scope but worth flagging):
- The CLI's "at least one
--origin" guard is stricter thanprovideHttpClient, which admits an empty allowlist. That forecloses the mint-then-widen flow the promised phase-2allowverb implies; relaxing it later is minor-compatible, so cheap either way — but the design's Phase-1 section should say which is intended. [proposed-rule: a phased CLI surface must record where phase N is deliberately stricter than the capability it fronts.] - Re-running
mkon an existing pet name silently rebinds (storeIdentifier,packages/daemon/src/pet-store.js:101-112), orphaning the prior client, which stays live under its old policy for anyone already holding it. Worth a line in the verb description untilrevokeexists. [rule: skills/gricean-maxims/SKILL.md]
Self-improvement: the recurring shape here is front-end advertises a wider contract than the validator accepts; next migrator pass I will diff every new flag's description against the callee's assertion helper before reading anything else.
locksmith
locksmith
Verdict: request-changes
Findings:
-
packages/cli/src/endo.js:838—--policy-mode tofu-autois offered with the help text "Policy mode: strict (default) or tofu-auto", which says nothing about what it does to the attenuation. Intofu-autothe origin allowlist stops being a bound:packages/exo-http-client/src/http-client.js:390returns{decision:'allow', decidedBy:'tofu-auto'}for any origin not on the list. Soendo http mk n --origin http://example.com --policy-mode tofu-automints a client that reacheshttp://127.0.0.1:*andhttp://169.254.169.254as readily as the named origin, while the verb one-liner atendo.js:814still advertises "a confined HTTP client capability under an origin-allowlist policy" andhttp-mk.js:43enforces "at least one --origin" as if the list were load-bearing. The attenuator does not narrow what the surface claims. [rule: roles/jurors/locksmith/AGENT.md § Operating norms, "does each attenuator narrow the surface it claims to"] -
packages/cli/src/endo.js:837— Phase 1 deliberately ships noinspectverb, so an operator who passes--policy-mode tofu-autohas no CLI-reachable way to see which origins the client auto-pinned (control.inspectstays host-side behindgetHttpClientControl). Authority that widens itself at runtime and cannot be reviewed from the surface that minted it should not be mintable yet: either refuse--policy-mode tofu-autountilinspectlands, or state the effect in the flag help, the changeset, and the design section. [proposed-rule: a CLI flag that relaxes a capability's static bound must name the relaxation in its own help text, and must not ship ahead of the verb that lets the operator audit what the relaxation admitted.] -
designs/cli-http-client.md:59— "the SSRF and flooding defenses ... are enforced by the confinement layer the client is built on, so they need no separate CLI plumbing at this phase" holds only instrict. The origin allowlist is the SSRF defense, and this PR's own flag turns it off. Qualify the sentence. [rule: skills/gricean-maxims/SKILL.md § Quality] -
packages/cli/test/http-mk-command.test.js— no test passes--policy-mode,--max-requests-per-minute, or--max-response-bytes, and the end-to-end test never fetches through the minted client. A regression in whichmkdroppedallowedOrigins(the daemon accepts an absent allowlist,packages/daemon/src/host.js:225) or emittedpolicyMode:'tofu-auto'when unset would pass every test here. Add one test that the registered client refuses an off-allowlist origin. [rule: skills/adversarial-tests/SKILL.md]
Notes (out of scope but worth flagging):
provideHttpClientsits onHostInterface(packages/daemon/src/interfaces.js:369) and notGuestInterface, soendo http mk --as <guest>fails at the guard rather than letting a guest mint its own outbound network authority. Correct, but unpinned: worth a test so a later move onto the guest interface goes red. [proposed-rule: a network- or credential-minting daemon method's host-only interface placement carries a regression test asserting the guest path is refused.]
Self-improvement: the locksmith brief's two recurring-finding paragraphs cover unhardened surfaces and if (readOnly) gating, but not this shape — a mode enum whose non-default member silently converts an allowlist into an audit log. Worth adding as a third recurring pattern: check every mode/level flag for the member that disarms the attenuator, and check whether the surface's own prose still claims the bound.
warden
Juror: warden
Verdict: request-changes
Findings
1. --policy-mode tofu-auto dissolves the origin allowlist, and nothing on the minting surface says so — must-fix
packages/cli/src/endo.js:837 advertises the flag as neutral: "Policy mode: strict (default) or tofu-auto". But packages/exo-http-client/src/http-client.js:390 shows tofu-auto auto-allows every origin absent from the allowlist (decision: 'allow', decidedBy: 'tofu-auto'), and there is no private-range/link-local guard anywhere in exo-http-client or http-confine — the allowlist is the whole SSRF defense. So endo http mk x --origin https://a.example --policy-mode tofu-auto mints an effectively unconfined outbound-network capability that reaches http://169.254.169.254/, loopback, and the internal network, while --origin reads as if it bounded it.
The prose compounds it, which is the recurring docs-as-attack-surface case: .changeset/cli-http-mk-phase-1.md calls it a "mode guard", and designs/cli-http-client.md (added § Landed CLI surface) asserts "the SSRF and flooding defenses … are enforced by the confinement layer … so they need no separate CLI plumbing at this phase" — the flag the same phase ships turns off the origin half of exactly that defense.
Fix: state the widening in the --help text (e.g. "tofu-auto auto-allows any origin on first use — the allowlist stops bounding the client"), and correct the changeset/design wording. Withholding tofu-auto from Phase 1 until inspect/revoke exist would also be defensible.
[rule: roles/jurors/warden/AGENT.md § Operating norms — docs that document an unsafe surface are must-fix]
[proposed-rule: a CLI verb that mints a capability must name, in its own --help, any flag that widens or dissolves the confinement the verb otherwise advertises]
2. Policy record crosses the capTP boundary unhardened — should-fix
packages/cli/src/commands/http-mk.js:47-57 builds a mutable policy and hands it to E(agent).provideHttpClient. It works only because packages/captp/src/captp.js:1117 does serialize(harden([prop, args])) for us — and that deep harden freezes commander's --origin accumulator array (endo.js:818) in place, an object this module does not own. Mirror commands/form.js:26 and the daemon's own normalizeHttpClientPolicy: harden({ allowedOrigins: harden([...allowedOrigins]), … }). Daemon-side re-validation makes this defense-in-depth, not a hole.
[rule: AGENTS.md § Hardened JavaScript (SES) conventions]
3. Comment misstates the guard — comment-only
http-mk.js:49-50 justifies the conditional spread with "its normalizer rejects an undefined numeric". It does not: packages/daemon/src/host.js:242-262 explicitly defaults on undefined (60 / 1 MiB). The spread is fine; the stated reason is wrong.
[rule: skills/gricean-maxims/SKILL.md — quality]
No unguarded-global, prototype-walk, or intrinsic-shadow issues; the test file's process.env writes match test/formula-collection.test.js precedent, and host-only placement of provideHttpClient in HostInterface is correct.
Self-improvement: the seat's leverage here came from following the flag's value into the enforcement layer rather than trusting the design doc's summary of its own defenses — for a capability-minting verb, read every enum member's implementation before accepting the doc's claim that confinement is handled downstream.
saboteur
Per-juror block — saboteur (PR #1014)
Verdict: request-changes
Findings
1. must-fix — --policy-mode tofu-auto deletes the origin allowlist the verb advertises. packages/cli/src/endo.js:826 (--policy-mode <mode>). Attack input: endo http mk x --origin http://127.0.0.1:1 --policy-mode tofu-auto. In that mode decide() returns an unconditional allow for any unlisted origin (packages/exo-http-client/src/http-client.js:390), and neither @endo/http-confine nor exo-http-client blocks loopback or link-local targets (grep for 169.254/localhost/private: no hits), so the minted cap can fetch http://169.254.169.254/… or any internal host — capped only by maxBindings and the rate limit. The new surface's own words claim the opposite: the option description says "under an origin-allowlist policy", and the changeset says "the SSRF … defenses … are enforced by the confinement layer". http-mk.js:47's ≥1-origin guard is cosmetic under this flag. Remedy (cheap): state in the --policy-mode description and the changeset that tofu-auto auto-allows any first-seen origin, i.e. the allowlist becomes a pre-approval, not a bound — or withhold tofu-auto until the phase that ships deny/revoke/inspect gives an operator a way to see and undo an auto-pin. [proposed-rule: a CLI verb that mints a capability must not describe a bound that one of its own documented flags removes; the escape hatch is named where the flag is defined.]
2. should-fix — numeric flags coerce with bare Number(); the failure surfaces unlocated. endo.js:814,820 (val => Number(val)). --max-response-bytes abc → NaN, --max-response-bytes 1_000 → NaN, --max-requests-per-minute '' → 0. Each is !== undefined, so it is spread into the policy, crosses CapTP, and returns as provideHttpClient: policy.maxResponseBytes must be a positive safe integer — naming neither the flag nor what the user typed, after a daemon connect. [rule: roles/jurors/saboteur/AGENT.md § Located-error discipline, analogous parsers.] Fix in http-mk.js beside the existing allowlist check, echoing flag and raw value.
3. should-fix — no negative-input coverage for the attack surface the verb owns. test/http-mk-command.test.js tests exactly one rejection (empty allowlist). The coerced numerics (2) and a path-bearing origin (--origin https://h/p) are untested. [rule: skills/adversarial-tests/SKILL.md.]
4. comment-only — re-mk under an existing name. provideHttpClient always formulates and storeIdentifier overwrites, so mk with a live name silently re-points it at a client with a different policy despite the idempotence its provide prefix implies. Sibling-consistent with provideShell; not this PR's to fix.
Mitigated (no change needed): credential-, case-, path-, query-bearing origins (http://u:p@h, HTTP://H, https://h/p) all die on parsed.origin !== origin (host.js:186); empty-string and duplicate origins rejected/harmless; tofu-prompt/tofu-attenuator refused by the normalizer; empty name segments by parsePetNamePath. No broad-try or bare-catch introduced.
Attacks: 11 — real 3, mitigated 7, out of scope 1.
breaker
Per-juror block
breaker
Verdict: request-changes
Findings:
-
--policy-mode tofu-autodissolves the allowlist the verb advertises, and nothing on the CLI says so.packages/cli/src/endo.js:829describes the mode flag as'Policy mode: strict (default) or tofu-auto'; the command description one line up claims the client is minted'under an origin-allowlist policy'; the changeset callspolicyModeone of the "guards". Butpackages/exo-http-client/src/http-client.js:390-394makesdecide()return{decision:'allow', decidedBy:'tofu-auto'}for any target. Attack:endo http mk x --origin https://api.example --policy-mode tofu-automints a client that will fetchhttp://169.254.169.254/latest/meta-data/on first contact — the exact SSRFdesigns/cli-http-client.md(added §, "the SSRF and flooding defenses … are enforced by the confinement layer") says the allowlist defends. Three surfaces describe a loosening knob as a tightening one, and no test covers it. Related: the "at least one--origin" check is CLI-local only —normalizeHttpClientPolicyacceptsallowedOrigins: undefined→[](packages/daemon/src/host.js:227), so empty-allowlist + tofu-auto is an unrestricted client any other front-end can mint. should-fix: state the widening in the flag help and the changeset. [proposed-rule: a CLI flag that relaxes a security bound must say so in its own--helptext, never only in a design document.] -
mkon an existing pet name silently rebinds and orphans a live, unrevocable client.provideHttpClient(host.js:927) is not get-or-create like itsprovide*siblings (comparemakeChildHost'sgetNamedAgentlookup,host.js:~1857): it always formulates, thenstoreIdentifierdeletes the prior mapping and returns without cancelling it (packages/daemon/src/pet-store.js:97-121). Attack:endo http mk api --origin https://a.examplethenendo http mk api --origin https://b.example. The first client stays incarnated (GC is off unlessENDO_GC=1,manager-node.js:42) and keeps outbound fetch, while its control facet lives only in thehttpClientControlForClientWeakMap keyed by the client cap (manager.js:1204) — reachable viagetHttpClientControl(client), and the host's only route to that client was the name just overwritten. The revocation story this PR defers to a later phase is therefore already unreachable for the displaced cap;httpMkprintsapias though nothing was displaced. WithENDO_GC=1the mirror-image bug applies: the overwrite silently collects a client a guest holds. should-fix: refuse an existing name inpackages/cli/src/commands/http-mk.jsabsent--force, with a test. [rule: skills/adversarial-tests/SKILL.md — invariant-attack category] -
The delegation this PR is built on is claimed three times and tested zero times.
http-mk.js:20-23, the changeset, and the design section all assert the daemon's normalizer is the sole validity authority and "a malformed origin or guard surfaces as its structured error on the CLI invocation". The one negative test (test/http-mk-command.test.js:98-103) exercises only the CLI-local guard, which never reaches the daemon. Precedent says daemon errors do reach stderr (test/trace.test.js:84), so this is coverage, not correctness: add at.throwsAsyncfor--origin https://a.example/v1and--max-response-bytes 0. should-fix [rule: skills/regression-evidence/SKILL.md]
Notes (out of scope but worth flagging):
--origin <url>help says "URL", butassertHttpClientOrigin(host.js:174-195) requiresnew URL(o).origin === overbatim — sohttps://api.example/(trailing slash, the browser-copy form) andhttps://api.example:443are both rejected. Loud, not silent, so comment-only; either normalize withnew URL(o).originCLI-side or say "origin (scheme://host[:port], no path)". [proposed-rule: flag help names the accepted grammar, not a looser superset of it.]t.notRegex(stdout, /controller-name/)(http-mk-command.test.js:60-61) passes against any output that never had the string, including empty — near-vacuous as a rework guard. [rule: skills/adversarial-tests/SKILL.md]- Numeric coercion
val => Number(val)(endo.js:817,823) is safe against the attacks I tried (NaN,Infinity,1e21,''all fail the daemon's safe-integer check);0x10silently means 16. Mitigated, no change asked.
Self-improvement: the sharpest attack this round came from following a deferred capability (revocation, punted to a later phase) back into the current diff and asking what the current diff can already make permanently unreachable — a phase boundary is not an excuse to skip an invariant, it is where the invariant is easiest to break. Worth adding to skills/adversarial-tests/SKILL.md as a named attack: phase-deferred authority made unreachable by a phase-1 verb.
purist
Reviewed the diff at origin/llm...HEAD (CLI-only: endo http mk, plus changeset and design-status prose), and cross-read the daemon/exo side the verb rides.
Seat: purist
Verdict: request-changes
Findings
-
must-fix —
--policy-mode tofu-autoun-bounds the allowlist, and nothing on the verb says so.packages/cli/src/endo.js:830-833describes the flag as "Policy mode: strict (default) or tofu-auto", and the changeset plusdesigns/cli-http-client.mdcall the result "a confined outbound-HTTP client capability under a host-curated policy". But intofu-autothe exo auto-allows every origin that is not on the list:packages/exo-http-client/src/http-client.js:390-394returns{ decision: 'allow', decidedBy: 'tofu-auto' }for any unknown target, capped only bymaxBindings. So the flag turns--originfrom a confinement bound into a pre-seed, and the operator readingendo http mk --helpgets no signal. Say it in the option help ("auto-pins any origin on first use;--originthen only pre-seeds") and in the Phase 1 design paragraph. [proposed-rule: a CLI flag that relaxes the confinement boundary of the capability being minted must name that relaxation in its own--helpline.] -
should-fix — a code comment asserts a far-side contract that is false.
packages/cli/src/commands/http-mk.js:51-53: "the daemon applies its own defaults otherwise (its normalizer rejects an undefined numeric)".normalizeHttpClientPolicyexplicitly defaultsundefined(packages/daemon/src/host.js:246,:258,:268), and the guardM.recordOf(M.string(), M.any())admitsundefinedvalues. Keep the conditional spreads if you prefer the record shape; drop the invented rationale. [rule:skills/gricean-maxims/SKILL.md, Quality] -
should-fix — the non-empty-allowlist invariant lives only in the front end.
http-mk.js:42-45throws on an empty list while its own comment names the daemon "the authority on policy validity"; the daemon accepts a missingallowedOriginsand normalizes it to[](host.js:225-238).[]plustofu-autois exactly finding 1's unbounded client. Either mark the CLI check as a convenience, or (follow-up, outside this diff) havenormalizeHttpClientPolicyrequire a non-empty allowlist undertofu-auto. -
comment-only — roadmap narration in shipped surfaces.
endo.js:805-807("Phase 1: mk only. allow/deny/revoke/inspect land in later phases.") stales on the next phase and is not actionable for an operator;test/http-mk-command.test.js:57-58asserts the absence ofcontroller-name/client-name, strings no shipped surface ever emitted. [rule:skills/gricean-maxims/SKILL.md, Relation/Quantity] -
comment-only — family divergence and prose style.
mkhost/mkguestlog the returned capability,mkdirlogs nothing;http-mk.js:58echoes back its own argument. Also the new changeset and design prose are em-dash-heavy. [rule:skills/em-dash-style/SKILL.md]
No passability defect. The policy record and parsePetNamePath array go out unhardened, but captp hardens arguments at serialize (packages/captp/src/captp.js:549), matching every sibling command; Number(val) option coercion matches the precedent at endo.js:515, and NaN fails closed at the normalizer.
Self-improvement: the purist brief's "reuse over re-implementation" axis kept pulling toward throw new Error vs @endo/errors, which the whole packages/cli/src/commands/ family already spells as a raw throw; worth a line in the brief that a house idiom local to the directory outranks the package-level preference, so the seat spends its budget on the confinement axis instead.
spec-keeper
My review is complete. Here is the per-juror block:
spec-keeper
Verdict: request-changes
Findings:
-
--policy-mode tofu-autosilently voids the allowlist, and nothing on the surface says so.packages/exo-http-client/src/http-client.js:390resolves any off-list origin to{decision:'allow', decidedBy:'tofu-auto'}. Soendo http mk x --origin http://a --policy-mode tofu-automints a client that can reach the whole internet, yetpackages/cli/src/endo.js:820describes the verb as "mint a confined HTTP client capability under an origin-allowlist policy" and the mode flag as only "Policy mode: strict (default) or tofu-auto". The claim is too strong for what the flag does. Narrow the Phase-1 surface: either drop--policy-modeuntil theinspect/revokeverbs can audit auto-pinned bindings, or state in the flag help, the changeset, anddesigns/cli-http-client.md§ Landed CLI surface thattofu-autoauto-allows any origin on first use and makes--originadvisory. [proposed-rule: a CLI flag that widens a capability's authority beyond its stated confinement must say so in its own--helptext, not only in the design doc.] -
--origin <url>is documented as a URL but must be an exact WHATWG origin serialization.packages/daemon/src/host.js:188requiresparsed.origin === origin. Verified against Node's URL:http://example.com/→http://example.com(reject, trailing slash),http://EXAMPLE.com→ lowercased (reject),https://x.com:443→ default port elided (reject),https://ünicode.example→ punycode (reject). Every one of those is what a user copies from a browser bar. Per URL § origin and host-to-ASCII, the ASCII serialization is canonical — sopackages/cli/src/commands/http-mk.js:52should map each--originthroughnew URL(v).originbefore assembling the policy (the daemon check stays as the authority), and the flag help atpackages/cli/src/endo.js:824should say "origin", not "URL". [proposed-rule: a CLI flag whose value must equal a spec-defined canonical serialization normalizes locally rather than forwarding the raw token to a remote validator.] -
The tests do not pin which origin spellings are accepted.
packages/cli/test/http-mk-command.test.js:73happens to usehttp://127.0.0.1:<ephemeral>, the one shape that round-trips origin serialization — so the suite would stay green through any of the rejections above. Add cases for a trailing slash and an explicit default port. Relatedly, the livehttp.createServerthere is dead scaffolding and its comment is false:normalizeHttpClientPolicyonly parses the string, never dials it, so the listener adds a port-bind flake and pins nothing. [rule: skills/regression-evidence/SKILL.md]
Notes (out of scope but worth flagging):
val => Number(val)atpackages/cli/src/endo.js:829,835inherits StringNumericLiteral (ECMA-262 §7.1.4.1), so0x40,1e3, andInfinityare accepted while1_000and typos becomeNaNand only surface as a daemon error after a round trip. A local positive-safe-integer check is cheaper and clearer. [proposed-rule: numeric CLI options validate locally rather than relying onNumbercoercion plus a remote rejection.]--policy-modecould use commander's.choices(['strict','tofu-auto'])for local fail-fast and self-documenting--help. [rule: skills/adversarial-tests/SKILL.md]t.regex(stdout, /\bhttp\b/)(line 30) would match anyhttp://in help output, and the twot.notRegexassertions at lines 47–48 pass vacuously. [rule: skills/regression-evidence/SKILL.md]- The CLI's empty-allowlist rejection (
http-mk.js:44) has no daemon counterpart —normalizeHttpClientPolicydefaultsallowedOriginsto[]— which contradicts the changeset's "the daemon's policy normalizer is the authority on validity". If that guard is load-bearing (it is, undertofu-auto), it belongs on the daemon where a non-CLI caller also gets it. [proposed-rule: a validation rule stated as belonging to a lower layer is enforced there, not duplicated only in one front-end.]
Self-improvement: the seat's WHATWG-URL lens paid off here in a way the ECMA-262 lens would not have — origin serialization (default-port elision, host lowercasing, punycoding) is the recurring trap whenever a CLI flag is described as a "URL" but compared against URL.prototype.origin. Worth carrying forward as a standing check on any allowlist flag.
wire-watcher
wire-watcher
Verdict: request-changes
Findings:
-
must-fix —
--policy-mode tofu-autodisables the origin allowlist, and the CLI presents it as a peer of the tightening knobs.packages/cli/src/endo.js:832-835offers--policy-mode strict|tofu-autoalongside--max-requests-per-minute/--max-response-byteswith no distinction; under tofu-autodecide()returns{decision:'allow'}unconditionally for any unknown target (packages/exo-http-client/src/http-client.js:388-393), soendo http mk pet --origin https://api.example --policy-mode tofu-automints a client that reaches all of the internet whilemkprints success and the typed origin. The project's own design says exactly this: tofu-auto "converts the allowlist into a write-once log … never the default for an HTTP client" (designs/trust-on-first-bind.md:121). Phase 1 ships noinspect/revokeverb, so the holder cannot see what got pinned or undo it. Ask: restrict phase 1 tostrictuntilinspect/revokeland, or label the flag as allowlist-disabling in--help, the changeset, anddesigns/cli-http-client.md:52— where it is currently described as one of the "guards". [rule:roles/jurors/wire-watcher/AGENT.md§ In-band-marker trust-bypass] -
should-fix — failure-mode catalog is one entry deep. The changeset claims "a malformed origin or guard surfaces as its structured error on the CLI invocation", but the only negative test is the CLI-local empty-allowlist check (
packages/cli/test/http-mk-command.test.js:98-102); nothing exercises the daemon rejection path. Untested and cheap: a path-bearing origin (https://x.example/api), an off-scheme origin, a non-canonical one (https://Good.Example, trailing slash, userinfohttps://a@good.example— all rejected byparsed.origin !== origin,packages/daemon/src/host.js:186-195), and a refused--policy-mode tofu-prompt(the "refused rather than silently degraded" claim athost.js:209-212). [rule:skills/adversarial-tests/SKILL.md] -
should-fix —
val => Number(val)(packages/cli/src/endo.js:821,827) is a loose parse. It accepts0x100000,1e6,' 10 ', and yieldsNaNfor1_000. No cap is bypassed —Number.isSafeIntegerathost.js:246,258catches NaN/Infinity — but the user sees the daemon's generic "must be a positive safe integer" instead of a flag-named parse error. Parse decimal integers CLI-side. [proposed-rule: a CLI numeric option parser rejects non-decimal-integer input at parse time and names the flag in the error, rather than relying on a downstream validator.]
Notes (out of scope but worth flagging):
- Verified positive: origin canonicalization plus the redirect re-check (
packages/http-confine/src/http-confine.js:423-425,redirect:'manual'at:547) do close the SSRF-by-redirect hole the changeset asserts — understrict. [rule:roles/jurors/wire-watcher/AGENT.md§ Check before trust] - The repeatable
--originaccumulator seeds a single[]at command-construction (endo.js:812-818); harmless today (onemain()per process, no in-process test caller), latent ifmainis ever invoked twice — a prior invocation's origins would widen a later mint's allowlist. [proposed-rule: commander repeatable-option accumulators are seeded per invocation, never from a literal captured at construction, when the accumulated value is a security policy.]
Self-improvement: the seat's in-band-marker axis generalized cleanly from a hash field to an enum value — a mode name that switches an allowlist off is the same shape as "alg": "none". Worth adding "a policy-mode enum whose non-default value disables the check the other flags tighten" to the brief's trust-bypass examples, since the CLI surface presented it as one more optional knob.
engine-realist
engine-realist — PR #1014 (feat(cli): endo http mk)
Verdict: request-changes
Findings
1. should-fix — the design section this PR writes calls the control facet the durable mutate/revoke authority; it is ephemeral per incarnation. designs/cli-http-client.md (added § Landed CLI surface) says the WeakMap-held control facet "which the later allow/deny/revoke/inspect verbs will drive". But packages/daemon/src/manager.js:3335-3362 builds the client with no onChange and restores no bindings, while the exo exposes exactly that seam (packages/exo-http-client/src/http-client.js:304,316,329 — "Reconstitute persisted bindings before any mutation wires up onChange") and the neighbouring git-credential maker (manager.js:3364+) does wire onRotate/onRevoke to persistence. Lifecycle consequence: every TOFU pin and every control.revoke() lives only in this incarnation; on daemon restart the client reconstitutes from the frozen formula policy un-revoked. A revocation that resurrects is the wrong side of a fail-safe. Ask: say so in the new § (ephemeral vs formula-durable), and name "persist control-side mutations, or make revoke a formula-level act" as a prerequisite of the phase-2 revoke verb. [proposed-rule: a design section that promises a later verb's authority must state which incarnation-lifetime that authority has — ephemeral, virtual, or formula-durable.]
2. should-fix — --policy-mode tofu-auto is advertised without saying it dissolves the allowlist. packages/cli/src/endo.js:836-839 offers the mode as a bare alternative to strict, under a command described as "under an origin-allowlist policy" (endo.js:812). In that mode any non-allowlisted target is auto-allowed on first contact (exo-http-client/src/http-client.js:388-394); --origin becomes a seed, not a bound, and the SSRF story the changeset credits to the confinement layer applies only to strict. Help text should state it, and http-mk-command.test.js should pin that wording alongside its existing --origin regex.
3. comment-only — Number(val) coerces the two guard knobs (endo.js:829,834). --max-response-bytes 0x100000 is silently accepted as 1048576; 1_000_000 becomes NaN, crosses CapTP, and surfaces as the daemon's generic "must be a positive safe integer" without naming the flag. The repo already owns a strict decimal parser with precisely this reject-set (packages/cli/src/number-parse.js; its test rejects f00, +1, 1.0, 01, NaN, Infinity). Precedent for Number(val) exists at endo.js:515, but that bounds a display list — these bake a durable security bound.
4. should-fix — the fixture server is ceremony, and the policy itself is unasserted. provideHttpClient only normalizes and formulates (daemon/src/host.js:927-942); nothing connects, so the listener at test/http-mk-command.test.js:61-73 buys nothing while adding a listen-permission dependency the repo guards elsewhere (test/_net-permission.js, used at test/demo/index.test.js:18) — under an EPERM sandbox this test fails rather than skips. Use a literal origin. Separately, the assertions cover only the echoed pet name and endo list, so a mk that shipped allowedOrigins: [] would still pass; the cheap evidence is a negative case (--origin http://x/path) proving the record reaches normalizeHttpClientPolicy. [rule: skills/regression-evidence/SKILL.md]
5. comment-only — module-scope process.env mutation (test/http-mk-command.test.js:17-27) copies formula-collection.test.js, but AVA worker threads share process.env; demo/index.test.js:22-27 shows the hermetic pattern ($({ env }) over a mkdtempSync root). Prefer it for the new file.
Not findings, checked: the unfrozen policy record is fine (CapTP hardens the message; mkguest.js is the precedent); parsePetNamePath before withEndoAgent correctly rejects a bad name without spinning a daemon connection.
Self-improvement: the "storage choice" axis paid off here only because I chased the unwired seam — next engagement I'll grep the exo's constructor options for persistence hooks the daemon maker omits before reading the diff prose, since an unwired onChange is invisible in the diff itself.
integrator
integrator
Verdict: request-changes
Findings:
- PR description ignores the upstream template. The body invents seven headings (
## What,## Why this, and why narrow,## What's included,## Deliberately dropped from #286, ...) and fills none of the sections.github/PULL_REQUEST_TEMPLATE.mdprovides. The omission that costs the reviewer is Security Considerations: this verb mints an outbound network authority under a policy, exactly the "what new authorities are introduced" case the section exists for. Refill section for section, keeping the SSRF/rate/byte bounds story under Security. must-fix [rule:skills/pr-formation/SKILL.md§ Use the upstream template, section for section] ## What's includedis a file-by-file inventory (five bullets, each a path), and## Deliberately dropped from #286is a second one. The diff is the file record; the body owes behavior and intent. The supersession story (#286's collision, the aborted rebase, the stale approval) compresses to two sentences under Why plus one Out-of-scope line for the un-carriedhttp-confinefix. should-fix [rule:skills/pr-formation/SKILL.md§ No file callouts, § Behavior and intent, not diff]- Title carries rework history.
feat(cli): endo http mk on the policy-based HTTP client: the "on the policy-based HTTP client" tail only disambiguates against #286's abandoned shape, which no futuregit logreader has. Preferfeat(cli): add endo http mk to mint a confined HTTP client. should-fix [rule:skills/pr-formation/SKILL.md§ Title] designs/README.md:408anddesigns/cli-http-client.md:8leave the status taxonomy. "Phase 1 landed (...)" is not one of the column's head words (Complete / In Progress / Not Started / Proposed / Reference / ...);In Progress (Phase 3)is the existing precedent for this exact case, and design-doc Status fields are bare taxonomy words in 148 of 151 files. The row also flips out ofProposedwithout syncing the "Last updated" line (:18) or the Totals tally (:451), which the README's own convention requires. should-fix [proposed-rule: a designs/README status flip uses a taxonomy head word plus optional parenthetical, and syncs the Last-updated and Totals prose in the same commit]http-mk-command.test.js:59-80stands up a live HTTP server no assertion needs.normalizeHttpClientPolicydoes pure URL-shape validation and never connects (packages/daemon/src/host.js:174-195), so the comment's "so the origin is a real ... origin the normalizer accepts" justifies machinery by narrative. A literalhttp://127.0.0.1:8080drops the listener and the teardown. should-fixhttp-mk.js:50asserts a false daemon contract: "its normalizer rejects an undefined numeric". It defaults them (60, 1 MiB,strict;host.js:239-268). The line is inherited frommount.js:33-35, where it is true of the mount guard. Keep the conditional spread, fix the reason. should-fix- Second commit is a fixup to the first (
fix(cli): drop redundant /* global process */).llmpreserves individual commits, sogit loggains a fix for a defect that never reached it. Squash. should-fix (overlaps packager)
Notes (out of scope but worth flagging):
- Axes walked clean:
mkis design-sanctioned (cli-http-client.md:91) and coheres withmkhost/mkguest/mkdir/mktmp; "Network" is the daemon's own tier name (host.js:929);http-clientfalls to thecapabilitiescatch-all so the CLI/space-chat inventory tables stay in sync unedited (list.js:88-95); the repeat-collector and isolation harness follow existing precedent (endo.js:231,formula-collection.test.js). - Forward-compose:
provideShell/provideGitRemote/provideBearerCredentialare policy-bearing mints with no CLI verb yet.http mkis the first front-end, so its flags-to-policy-record shape is what the next three will paste. Naming it as the intended pattern in the design costs one sentence now. [proposed-rule: the first CLI front-end for a capability tier names the pattern the sibling tiers will extend] cli-http-client.md:160-173still lists positional<origins...>andset-rate/set-bytes/set-time, now folded intomkflags; the new section hedges ("differs from the placeholder tables below") without reconciling the verb list the help text advertises.
Self-improvement: the seat's convention-probe axis paid off twice by checking whether the de-facto practice matches the standing rule before grading. Sampling merged PR bodies showed free-form prose is the norm on llm, which kept a template finding from being overstated into a process complaint, while the status-column tally showed the taxonomy really is load-bearing. Worth encoding in the brief: probe the last three merged instances before scoring a convention deviation.
benchmarker
benchmarker
Verdict: approve
Claim inventory (PR body, both commit messages, the diff's prose, and the PR's threads; gh reports zero comments and zero reviews on #1014 as of this round, so no thread-level claim is open):
- PR body § "
http-confinechange from #286: assessed, not carried" declines carrying #286's inert-response-snapshot change. Closed by an explicit not-pursuing rationale, and a good one: it names the reason (unverified relevance to thellmcode path, CI pinned to Node 24 where the slot is not tripped) and the closure condition (land separately with a Node-22 reproduction). This is the shape the seat exists to see. designs/cli-http-client.md§ Landed CLI surface (Phase 1): "the SSRF and flooding defenses (redirect: 'manual', the per-response byte cap, the sliding-window rate limit) are enforced by the confinement layer the client is built on, so they need no separate CLI plumbing at this phase." Closed by delegation with rationale. I checked the delegation is accurate rather than assumed: the guards and their defaults are real one layer down (packages/exo-http-client/src/http-client.js:728-751,packages/http-confine/src/types.ts:19,33-34), andhttp-mk.jsomits unset knobs so those defaults apply.packages/cli/src/endo.js:848lazy-imports the verb module. That is a startup-latency pattern, but it is not a new claim: all 57 command wirings in the file already do it. No closure owed.- No
BENCH.mdadded or updated, and no optimization is proposed in either commit message.
Findings:
packages/cli/src/endo.js:827,832newly expose two quantitative resource caps (--max-requests-per-minute <n>,--max-response-bytes <n>) whose effect is never exercised end to end in this PR: the one daemon-driven test registers a name with--originonly. Nothing here is wrong for Phase 1 and the deferral is deliberate, so a post-merge measurement suffices. Disposition:follow-up. [proposed-rule: when a CLI newly exposes a quantitative resource cap, the PR closes it with one end-to-end demonstration that a chosen value takes effect, or an explicit deferral note naming who measures it and when]
Notes (out of scope but worth flagging):
- None. Validation of
Number(val)coercion on those two flags is the prover/corner-prober lane, not this seat's.
Self-improvement: both rule paths my brief instructs me to cite, skills/benchmark-comparative-report/SKILL.md and roles/scout/AGENT.md § When dispatched, do not exist in the active library (roles/scout/ and skills/benchmark-comparative-report/ are absent, and neither appears in CLAUDE.md § Current inventory). Every benchmarker finding citing them would be dropped at aggregation for citing a nonexistent path, per skills/panel-review/SKILL.md § Cite-or-propose discipline. The seat needs either those two artifacts written or its citation instruction repointed; this round I fell back to [proposed-rule]. Worth a message to the gardener.
changeset-auditor
Juror seat: changeset-auditor — PR #1014
Verdict: request-changes (summary-fix only; nothing blocks un-draft)
Coherence checks that pass — recorded so the fixer does not churn them:
- Package-set: front-matter lists
'@endo/cli'only; the diff touchespackages/cli/{src/commands/http-mk.js,src/endo.js,test/http-mk-command.test.js}plusdesigns/(not a package). No missed or stale entry. - Bump level:
minorfor a purely additive CLI verb on an existing package (@endo/cli@2.3.13) matches the in-tree precedent.changeset/cli-mount-denied-segments.md. Not a new package, so § New-package initial release does not apply. - Body-vs-diff identifiers:
provideHttpClient(name, policy),getHttpClientControl,normalizeHttpClientPolicy, andstrict|tofu-autoall match the tree (packages/daemon/src/host.js:162,217,927,946); the usage block's flags matchendo.jsexactly. No stale draft language. - Bundling: one changeset for the PR. ✓
Findings
-
[summary-fix]
.changeset/cli-http-mk-phase-1.md(paragraph beginning "It rides the HTTP client that already landed…") — the body narrates the PR's own scope and design history: "rides the HTTP client that already landed on the daemon rather than introducing a new formula", "not the controller/client formula pair the original design assumed", "This is CLI-only; no@endo/daemonchange is needed." A downstream package author reading@endo/clirelease notes needs the new affordance and its flags, not which formula was reused or which sibling package went untouched. Reduce to the user-visible facts: the verb mints a confined client under an origin allowlist with optional rate/size/mode guards, registers it under a pet name, and surfaces the daemon's structured error on a malformed policy. [rule:skills/changeset-discipline/SKILL.md§ What goes inside — Omit implementation details, No process commentary] -
[summary-fix]
.changeset/cli-http-mk-phase-1.md:5and the closing sentence — "Phase 1 ofdesigns/cli-http-client.md" and "theallow/deny/revoke/inspectverbs, follow in later phases" are repo-internal roadmap pointers;designs/does not ship, so the published entry cites a path its reader cannot open, and future verbs get their own entries when they land. Drop both; keep the phase framing in the PR body. [rule:skills/changeset-discipline/SKILL.md§ What goes inside — audience is "a downstream package author reading the published release notes"]
Not raised: the hard-wrapped (rather than sentence-per-line) body — the skill carries no such sub-rule and every neighboring .changeset/*.md wraps the same way; imposing it here would be an internal-convention sweep per skills/panel-review/SKILL.md § calibration.
Adjacent, outside this seat's lens (flagged for the packager/pedant seats): packages/cli/src/commands/http-mk.js:47 comments that the daemon's "normalizer rejects an undefined numeric" — it does not; normalizeHttpClientPolicy defaults undefined to 60 / 1 MiB (host.js:241,254). The conditional spread is still correct; the comment's justification is wrong.
Self-improvement: this seat's brief cites a § Sentence-per-line section that skills/changeset-discipline/SKILL.md does not contain — a dangling citation of the same species as the § Bump level for new packages one repaired 2026-06-26. Either add the section to the skill or drop it from the brief; leaving it invites a seat to cite a rule that does not exist.
surfacer
surfacer
Verdict: request-changes (must-fix-loop)
The PR adds one new top-level CLI verb group (endo http, Phase 1 mk). The @endo/cli package has "exports": {} and no index thunk or .d.ts, so the four-way lens collapses onto the CLI's user-facing surface: commander's --help tree, the option help text, the design doc, and the repo's enumerated CLI reference. Three of those disagree with what the daemon actually accepts.
Findings
-
--origin <url>advertises URLs; the callee accepts only bare origins.[must-fix-loop]packages/cli/src/endo.js:820names the metavar<url>and describes it as "Allowed origin URL (http: or https:)";packages/cli/src/commands/http-mk.js:26says "Allowed origin URLs". ButassertHttpClientOrigin(packages/daemon/src/host.js:175) requiresparsed.origin === originverbatim: no path, query, fragment, and no trailing slash, sohttps://api.example.com/v1and evenhttps://api.example.com/are refused. The CLI's own claimed surface should describe the shape the daemon takes: metavar<origin>, help "scheme://host[:port]; no path, query, or trailing slash". [proposed-rule: a CLI flag's metavar and help text must describe the value shape the receiving method actually accepts, not a looser superset.] -
The comment justifying the conditional spread misstates the daemon contract.
[must-fix-loop]packages/cli/src/commands/http-mk.js:51says the guard knobs are omitted because "its normalizer rejects an undefined numeric".normalizeHttpClientPolicydoes the opposite:undefineddefaults to 60 and 1 MiB (packages/daemon/src/host.js:242-256). Omitting is still right (it lets the daemon own the defaults, which is what the changeset and design say), so only the parenthetical is wrong. [rule:roles/jurors/curator/AGENT.md, surface documentation must match the surface.] -
The new verb group is absent from the CLI reference the agent stack reads.
[must-fix-loop]packages/lal/primer/cli-reference.mdenumerates every verb group (Daemon Management through Agents) andpackages/lal/prompts/system.js:76instructs agents to read it. A new "Network" group inendo --helpwith no matching section leavesendo http mkundiscoverable there. Three lines. [proposed-rule: a new top-levelendoverb group lands with its entry inpackages/lal/primer/cli-reference.md.] -
--as <agent>on a host-only method.[follow-up]provideHttpClientsits inHostInterface(packages/daemon/src/interfaces.js:369), notGuestInterface, so--as <guest>fails with a method-missing error. Uniform with every other command's--as, so not this PR's to fix, but worth a help-text note when the later verbs land.
Coherent and worth noting as correct: --policy-mode strict|tofu-auto matches HTTP_CLIENT_POLICY_MODES exactly, including the deliberate exclusion of tofu-prompt/tofu-attenuator; the design's new "Landed CLI surface" section correctly demotes the old controller/client-pair tables to historical context.
Self-improvement: the CLI-shaped case has no exports/thunk/.d.ts, so the seat's four surfaces map to help tree, flag help, design doc, and enumerated reference doc. Reading the callee's validator as the authoritative surface (rather than only the package's own files) is what caught findings 1 and 2; I will start there on any CLI front-end diff.
scribe
Per-juror block: scribe
Verdict: comment-only.
Surface walked. #1014 has zero review comments, zero issue comments, zero reviews (fresh draft; both commits predate any review), so no note-this ask originates here. The PR is the carry-forward of #286, so I walked #286's history: 5 issue comments, 2 reviews.
Findings
1. The maintainer's one note-this directive is closed — but this PR's diff edits the very doc that carries the closure and never names it. #286 review 4943057191 (kriskowal, APPROVED, 2026-08-15): "Please post a follow-up job to elaborate on this HTTP client and controller system to allow for metering, fees, rate limiting, retries, and circuit breaking… note also the recent design direction for metering the minion.town gateway." Closure exists and is good: journal jobs/tada/endojs-endo-but-for-bots-http-adapter-pipeline-design.md → designs/http-adapter-pipeline.md, draft PR #992 (open, 2026-08-15), which also adds a forward pointer from designs/cli-http-client.md and a designs/README.md row. This PR rewrites the same header block (Updated, Status), the same README row, and inserts a 51-line Landed CLI surface (Phase 1) section whose forward plan names only allow/deny/revoke/inspect — no mention of metering/fees/retry/circuit-breaking or #992. Whichever lands second silently overwrites the other's record of the phase plan. Disposition: summary-fix — one sentence plus a relative link to designs/http-adapter-pipeline.md (PR #992) in the new section, naming it as the elaboration track the approval directed. [rule: skills/panel-review/SKILL.md § Cite-or-propose]
2. An assessed-and-dropped fix has no record outside a PR body. The PR body's "http-confine change from #286: assessed, not carried" ends "Recommend landing it separately on its own merits with a Node-22 reproduction." That recommendation exists nowhere durable: #286 comment 4930337643 documents the reproduced+verified fix on a branch that is now closed; #286's closing comment 5312129933 does not mention it; no journal job (grep -il http-confine jobs/{todo,plan,doin} → none), no issue, no open PR carries it. Branch deletion strands it. Disposition: summary-fix — post a follow-up job or file an issue before un-draft. [proposed-rule: a change deliberately assessed-and-not-carried out of a superseded PR must produce a durable record — a posted job, an issue, or a design note — outside the successor PR's description, since the description is the least discoverable place a future builder looks.]
Completion-summary closure: clean. #286's "This needs a refresh and conduct" (#issuecomment-5300875299) drew an ack (5300881172) and then a top-level comment (5312129933) naming the collision, what was superseded, and where the work moved. No directive-responding push on #1014 yet, so none is owed here.
Self-improvement: the scribe's brief presumes the note-this asks live on this PR; for a supersession successor they live on the predecessor, and the sharper lens is whether the successor's diff preserves the predecessor's closure records rather than overwriting them. Worth adding to the brief as a standing sub-check.
pruner
pruner (documentation padding)
Verdict: request-changes
Findings:
-
.changeset/cli-http-mk-phase-1.md:16-24,26— the changeset spends 11 of its 24 body lines on internal design history ("It rides the HTTP client that already landed on the daemon rather than introducing a new formula", "not the controller/client formula pair the original design assumed", "This is CLI-only; no@endo/daemonchange is needed"). This text is published verbatim into@endo/cli'sCHANGELOG.mdand outlives the design doc; a consumer cannot act on which plan the implementation deviated from. Cut lines 16-22 (through "original design assumed") and the first sentence of 26; keep 5-14 and the one consumer-relevant sentence — a malformed origin or guard surfaces as the daemon's structured error. [rule:CONTRIBUTING.md§ Writing the Changeset Body — "Describe the change from the consumer's point of view"] -
designs/cli-http-client.md:27-37and:64-72— both paragraphs restate the supersession blockquote that already sits ten lines above them at:13-24(formula pair superseded; facet split and method placement remain normative;endo httpverb tree survives). Delete both; the section's payload is:39-62(theprovideHttpClient(name, policy)signature, the policy record, the landed flags), which is genuinely new. Relatedly,:70-72annotates the stale mint/formula-type sketches as "retained only as historical design context" — annotating superseded material is the padding move; delete the sketches (:364,:519-520) instead. [proposed-rule: a design doc records supersession once, in one place, and deletes superseded sketches rather than labelling them historical] -
packages/cli/src/commands/http-mk.js:8-25— 17 lines of prose JSDoc above a 25-line function, restating the changeset a second time. The sibling command with identical shape,packages/cli/src/commands/mount.js:9-20, is one purpose line plus@paramtags. Cut to one line plus the existing@paramblock; the inline comment at:47-48already carries the only non-obvious fact (why guards are omitted when unset). [rule:packages/cli/src/commands/mount.js:9-20in-repo precedent] -
packages/cli/src/endo.js:799-803— a 5-line phase-narrative preamble, the only multi-line comment in an 1100-line file (:47,:58,:1039are single terse lines). Delete; the design link belongs in the changeset. [proposed-rule: change-history narrative belongs in the commit message or changeset, not in permanent source comments] -
packages/cli/src/endo.js:807-808— theendo http --helpdescription embeds the project roadmap ("Phase 1: mk only. allow/deny/revoke/inspect land in later phases."). A CLI user needs the verbs that exist, not the phasing plan; no other.description()in the file embeds a newline. Drop the second line. [proposed-rule:--helptext states current capability, never roadmap]
Notes (out of scope):
packages/cli/test/http-mk-command.test.js:50-51is the fifth restatement of the same "policy, not a name pair" fact; thet.notRegexassertions already say it. Trim to one clause. [proposed-rule as above]
Self-improvement: the strongest pruner leverage this round came from asking where does this text live forever — a changeset publishes into CHANGELOG, so implementer-POV prose there costs more than the same prose in a design doc. Worth encoding in the brief as a durability ranking: changeset > source comment > design doc.
gateway
Per-juror block — seat: gateway — PR #1014 (endojs/endo-but-for-bots), base origin/llm, head 32e4d80981
Verdict
approve
Primary-surface walk
Every path in the seat's watch list was checked against the diff; none is touched:
.changeset/cli-http-mk-phase-1.md (release metadata, not repo config)
designs/README.md (docs index)
designs/cli-http-client.md (docs)
packages/cli/src/commands/http-mk.js (new)
packages/cli/src/endo.js
packages/cli/test/http-mk-command.test.js (new)
No root package.json, no yarn.lock/pnpm-lock, no tsconfig*.json, no .eslintrc*/eslint.config.*, no .github/workflows/*, no .config/*, no root prettier/editorconfig/gitattributes/gitignore. Confirmed by path filter over git diff --name-only origin/llm...HEAD.
Corroborating checks that a root-config touch was genuinely unnecessary rather than merely omitted:
- No undeclared dependency.
http-mk.jsimportsos(builtin) and@endo/eventual-send, already a declared dependency ofpackages/cli/package.json:37. The test usesexeca(devDep),ava,node:http. So no root or package manifest edit — and nodepcheck.ymlfallout — is owed. - No CI wiring owed. The new test matches the existing
ava.filesglobtest/**/*.test.*(packages/cli/package.json), so.github/workflows/ci.ymlneeds no edit. - No ignore-rule owed. The test's scratch root
packages/cli/test/tmp/is already ignored bypackages/cli/test/.gitignore:1and root.gitignore:139. - Changeset scope is legitimate.
@endo/cliisprivate: true, and.changeset/config.jsonsetsprivatePackages: { version: true, tag: true }; six prior@endo/clichangesets exist inorigin/llmhistory.minormatches a new verb. - Justification present and specific. The changeset and
designs/cli-http-client.md§ Landed CLI surface (Phase 1) both state what changed, the scope (CLI-only, "no@endo/daemonchange is needed"), and why the surface diverges from the original design. The claim checks out:provideHttpClient(petName, policy)exists atpackages/daemon/src/host.js:927with the normalizer at:219–271, unmodified by this PR.
Findings
designs/README.md:408— status flip asserts landing with no evidence pointer.[proposed-rule: a designs/README status row flipped to a landed/implemented state names its evidence — PR number or merge commit — as the neighboring rows do]. The row now reads "Phase 1 landed" while the change is still an unmerged PR, and unlikebreak-dev-dependency-cycles((on \llm`), line 407) it cites neither branch nor PR. Sibling precedent is mixed (cbor-codec:401says "Phase 1 implemented" bare), so this is a nit, not a gate. Disposition: **summary-fix** — append the PR reference and, if the convention is meant to hold, the(on `llm`)` branch qualifier.
No relaxation, no per-package-vs-root scope question, and no security-relevant config loosening arises in this diff. The one security-shaped element — a new outbound-network capability — is confined by the daemon-side normalizer and the @endo/http-confine layer rather than by any repo-wide config knob, which is the correct placement for this seat's purposes.
Self-improvement: the seat's watch list is worth extending with .changeset/config.json and per-package test/.gitignore, since "was a root-config touch actually owed?" is answered by those two files as often as by the ones already listed; I had to derive both by hand here.
corner-prober
Juror: corner-prober — PR #1014
Verdict: request-changes
Boundary enumeration (closure state). Covered: zero-origin allowlist (local guard, test rejects an empty origin allowlist); single-origin happy path; help-surface strings. Missing: every other corner below.
F1 — Origin exact-serialization boundary is unnormalized, undocumented, untested — must-fix-loop
assertHttpClientOrigin (packages/daemon/src/host.js:186) requires parsed.origin === origin verbatim. So --origin https://example.com/ (the trailing-slash form a browser URL bar and new URL(x).href both yield), --origin HTTPS://Example.com, and --origin https://example.com:443 are all rejected — while the flag help (packages/cli/src/endo.js:819) promises "Allowed origin URL (http: or https:)", and every one of those is a valid URL. The single likeliest first user action fails. Fix cheaply: normalize in httpMk (new URL(o).origin), or say "origin only — no trailing slash, path, or default port" in the help; either way add a test pinning the case. [rule: skills/adversarial-tests/SKILL.md § Boundary sweep — strings/normalization]
F2 — The claimed round-trip error path has no test — summary-fix
Both the changeset and packages/cli/src/commands/http-mk.js:21 claim "a malformed origin or guard surfaces as its structured error on the CLI invocation". No test exercises it; the only negative test hits the local pre-daemon guard, so the daemon→CLI error surface (message on stderr, nonzero exit) is entirely unpinned. Add one t.throwsAsync inside the existing serial daemon test. [rule: skills/regression-evidence/SKILL.md]
F3 — Number(val) coercion corners — summary-fix
val => Number(val) (endo.js:832,838) admits: abc→NaN, ''→0, 0x10→16, 1e3→1000, 60.5, -0, 2**53. All are bounded by the daemon normalizer, so these are test gaps, not bugs — except --max-requests-per-minute -5, which Commander reads as an option token and rejects with "argument missing", never reaching validation. Pin at least NaN and negative. [rule: skills/adversarial-tests/SKILL.md § Boundary sweep — numbers]
F4 — Identity collision: re-mk under an existing name silently rebinds — summary-fix
provideHttpClient unconditionally formulates, and pet-store.js:101-114 overwrites without error. endo http mk foo --origin A then --origin B rebinds foo to a second, differently-bounded network capability; the first stays incarnable and unrevoked, and mk prints identical output both times. Pin the intended semantics. [proposed-rule: a verb minting a network-authority capability must test the name-collision case explicitly]
F5 — minor — comment
Duplicate --origin entries persist verbatim (host.js [...allowedOrigins]) though the exo dedups into a Set (http-confine.js:173), so equivalent policies yield distinct frozen records. --origin '' passes the local arity-only guard and fails only remotely.
Self-improvement: the enumeration that paid here was string normalization vs. string equality on a user-typed identifier — when a validator compares a parsed value's canonical serialization against the raw input, the human-facing layer owns the normalization, and the copy-paste form is the first case to check.
fast-checker
Juror: fast-checker — PR #1014
Verdict: request-changes (one must-fix-loop item; rest summary-fix / follow-up)
Findings
1. --origin repeatability and all three guard knobs are forall claims with zero coverage. [must-fix-loop] — packages/cli/src/endo.js:820-838, packages/cli/test/http-mk-command.test.js
The changeset and help text both claim --origin <url> [--origin <url>...] plus --max-requests-per-minute, --max-response-bytes, --policy-mode. Every behavioral test passes exactly one --origin and no guard flag; the n>1 accumulator and all three coercions are unexercised. The record-assembly contract is quantified: for all knob-supply combinations, the policy contains exactly the supplied keys, and allowedOrigins deep-equals the flag sequence in order. Make it testable by exporting the pure assembler from http-mk.js (e.g. makeHttpClientPolicy({allowedOrigins, maxRequestsPerMinute, maxResponseBytes, policyMode})) — the current code interleaves it with withEndoAgent, so no unit property can reach it — then:
fc.assert(fc.property(
fc.array(fc.integer({min:1,max:65535}).map(p=>`http://127.0.0.1:${p}`), {minLength:1}),
fc.option(fc.integer({min:1,max:2**31}), {nil: undefined}),
fc.option(fc.integer({min:1,max:2**31}), {nil: undefined}),
fc.option(fc.constantFrom('strict','tofu-auto'), {nil: undefined}),
(origins, rpm, bytes, mode) => {
const p = makeHttpClientPolicy({allowedOrigins: origins, maxRequestsPerMinute: rpm, maxResponseBytes: bytes, policyMode: mode});
t.deepEqual(p.allowedOrigins, origins); // order + arity preserved
t.is('maxRequestsPerMinute' in p, rpm !== undefined); // omitted-when-unset
t.is('maxResponseBytes' in p, bytes !== undefined);
t.is('policyMode' in p, mode !== undefined);
}));Plus one example-based CLI test with two --origin flags. [rule: skills/regression-evidence/SKILL.md] — the omitted-when-unset comment at http-mk.js:50 is load-bearing (an included undefined is rejected by the normalizer) and currently unasserted.
2. Equivalent-implementations property across the CLI/daemon seam. [summary-fix] — packages/cli/src/endo.js:826,832
val => Number(val) vs the daemon's Number.isSafeInteger(n) && n > 0 are two predicates for one contract. fc.assert(fc.property(fc.string(), s => { const n = Number(s); return coerce(s) === n && (Number.isSafeInteger(n) && n > 0) === cliAccepts(s); })) falsifies at once and shrinks to the minimal witnesses ''→0, '0x10'→16, '3.5', '1e400'→Infinity, 'abc'→NaN. None corrupt state (the normalizer rejects all), but each costs a live daemon round trip to report what is a CLI usage error. Property-test a shared parsePositiveSafeInteger in the CLI so the two predicates provably agree. [proposed-rule: a CLI flag coercion that mirrors a daemon-side validity predicate must be property-tested for agreement with it, not spot-checked]
3. fc is not a devDependency of @endo/cli. [follow-up] — packages/cli/package.json
fast-check is already in-house (packages/agentry, packages/chacha12-fast-check-test), so only the per-package dep is missing. [proposed-rule: introduce fast-check as a devDependency on packages that ship universally-quantified contracts]
4. Delegated authority, example-only coverage. [follow-up] — the changeset makes normalizeHttpClientPolicy "the authority on validity", so its coverage is now load-bearing for this verb. packages/daemon/test/http-client.test.js is entirely example-based; a property over fc.webUrl() would pin the accepted origin set (notably that a browser-copied http://x.example/ is rejected for parsed.origin !== origin). Out of this diff, hence follow-up.
No round-trip or algebraic-identity shapes in this diff; the assembler is a projection, not a codec.
Self-improvement: the strongest fast-checker lever on a CLI diff is arguing for the extraction that makes a forall reachable at all — a property proposal against a function fused to its I/O is unactionable, so name the pure seam first, then the arbitrary.
releaser
Reviewed the diff (changeset, design doc, packages/cli/src/commands/http-mk.js, endo.js wiring, test) and cross-checked the daemon surface it calls.
Juror: releaser — PR #1014 (endojs/endo-but-for-bots)
Verdict: comment-only
A changeset is warranted here (new public CLI verb endo http mk, a capability the upgrading user can start using), and '@endo/cli': minor is the right bump for an additive verb with no migration. No bump-mismatch, no absent-but-required finding. The two findings below are both audience/accuracy defects in the body, disposition summary-fix.
F1 — Changeset body addresses the design reviewer, not the upgrading user (summary-fix)
.changeset/cli-http-mk-phase-1.md, paragraphs 3–4. Roughly half the entry narrates internals and PR scope: provideHttpClient(name, policy), "backed by @endo/exo-http-client over @endo/http-confine", the control facet "held host-side (reachable via getHttpClientControl)", "not the controller/client formula pair the original design assumed", "the daemon's policy normalizer is the authority on validity", and "This is CLI-only; no @endo/daemon change is needed." The last is pure scope commentary about the PR; the design-delta sentences are already stated near-verbatim in designs/cli-http-client.md § Landed CLI surface, which is where they belong. (Phase 1 of designs/cli-http-client.md) also points a published-CHANGELOG reader at a repo-internal document they cannot follow from npm.
[rule: skills/changeset-discipline/SKILL.md § What goes inside — "Omit implementation details", "No process commentary"]
Keep paragraph 1 + the usage block; state the guard defaults the user actually needs (--policy-mode defaults to strict; the unset guards default to 60 requests/minute and a 1 MiB response cap, per packages/daemon/src/host.js:241-255); and keep the last clause reframed as a present limitation ("policy mutation and revocation are not yet exposed on the CLI") rather than a phase plan.
F2 — Release note advertises --as <agent> without its host-only restriction (summary-fix)
The usage block offers [--as <agent>] unqualified, but provideHttpClient is declared only on HostInterface (packages/daemon/src/interfaces.js:369) and is absent from GuestInterface — so endo http mk … --as <a-guest> fails on the interface guard. A CHANGELOG reader will take the flag at face value. In-tree precedent for saying so plainly: .changeset/formula-inspector-getformula.md — "getFormula is absent on the guest facet". Add one clause: the named agent must be a host.
[rule: skills/changeset-discipline/SKILL.md § What goes inside — cite the user-visible fact, the affordance and its shape]
Self-improvement: no new seat guidance needed; both findings fell out of existing changeset-discipline clauses. Worth noting for the seat that checking a documented flag against the daemon's interface guard (host vs guest) is cheap and catches a class of misleading release notes.
transplanter
Juror: transplanter — PR #1014 (endojs/endo-but-for-bots)
Verdict: request-changes (two should-fix; both test-only — shipped code packages/cli/src/commands/http-mk.js and src/endo.js are portability-clean: no home dirs, hostnames, /run/user/<uid>, absolute install paths, GNU-only flags, or shell literals; withEndoAgent(agentNames, { os, process }, …) matches the house injection form.)
Findings
1. should-fix — packages/cli/test/http-mk-command.test.js:78,101: the two test.serial cases invoke a bare endo off PATH.
const cli = $({ cwd: dirname }) + cli`endo purge -f` resolves endo through execa's preferLocal, so it depends on a bin link existing in the runner's node_modules/.bin. Verified in this worktree: both fail with spawn endo ENOENT, and the empty-origin assertion then compares '' against /at least one --origin/ — the test cannot distinguish "the CLI rejected the empty allowlist" from "endo was not found". On a contributor's box with a globally installed endo it silently exercises that build, not the checkout. This file already contains the portable form 20 lines above (execa(process.execPath, [endoBin, '--help']), endoBin derived from import.meta.url). Fix: use it for the empty-origin case (it never reaches the daemon, so it needs no $ at all), and for the daemon case spawn cli`${process.execPath} ${endoBin} purge -f`. Pre-existing convention — test/formula-collection.test.js:32 fails identically here — so the pattern isn't this PR's, but this PR shouldn't extend it while importing the portable form. [rule: roles/jurors/transplanter/AGENT.md — "a tool assumed on PATH without a fallback"] [proposed-rule: a CLI test must invoke the binary under test via a path derived from import.meta.url + process.execPath, never a bare PATH name, so it cannot bind to a globally installed build]
2. should-fix — test/http-mk-command.test.js:59-67: binds a listening TCP socket with no netListenAllowed gate.
server.listen(0, '127.0.0.1') is exactly the case packages/cli/test/_net-permission.js exists for; test/demo/index.test.js:18 gates on it (netListenAllowed ? test.serial : test.serial.skip). Where listen() is EPERM this new test fails instead of skipping. Nothing ever connects to the server — normalizeHttpClientPolicy (packages/daemon/src/host.js:217) validates origin shape only — so the cheapest fix drops it entirely and passes a literal --origin http://example.test; otherwise gate it like demo/index.test.js. [rule: packages/cli/test/_net-permission.js + test/demo/index.test.js:18] [rule: roles/jurors/transplanter/AGENT.md — single-host network assumptions]
Checked clean: ENDO_SOCK derives from os.tmpdir() + process.pid with a file-unique prefix; XDG_* derive from import.meta.url; test/tmp/ is gitignored (.gitignore:139) and the run left the tree clean.
coverage-auditor
coverage-auditor
Verdict: comment-only
Findings:
- coverage of new lines could not be verified: no c8 coverage report at '/home/kris/garden2/scratch/project-wt-endojs-endo-but-for-bots-pr286-cli-verb-rework-gauntlet-panel-1-ffbd353c/coverage/coverage-final.json' (run c8 with --all --reporter=json, or set GARDEN_COVERAGE_JSON); cannot verify new-line coverage — NOT assuming covered. Produce a c8 report (
c8 --all --reporter=json) so new-line coverage can be checked, or confirm this package is intentionally outside coverage. This is surfaced, NOT treated as covered. [rule: skills/coverage-driven-testing/SKILL.md]
model claude-opus-4-8 · harness claude · garden 33127fab
Apply the panel's must-fix items on PR #1014: - Document the --policy-mode tofu-auto widening on the minting surface itself (flag --help, changeset, design doc): it auto-allows any first-seen origin, so --origin only pre-seeds and the allowlist no longer confines outbound reach. Prefer strict until inspect/revoke land. - Validate flag lexical shape locally, reporting by flag name: --origin is normalized to its canonical WHATWG origin (accepting browser-copied trailing-slash / default-port forms), the numeric guards must be positive integers, and --policy-mode must be strict|tofu-auto. Semantic policy validity still defers to the daemon normalizer. - Fix the --origin metavar from <url> to <origin> and describe the exact shape the daemon accepts. - Extract a pure makeHttpClientPolicy assembler (packages/cli/src/http-mk-policy.js) and unit-test the record shape, omitted-when-unset knobs, origin order, normalization, and the numeric-flag parser — the policy is now observed, not just the echoed pet name. - Drop the dead live-HTTP-server fixture (nothing dialed it) and the inline import() JSDoc type; invoke the CLI under test via process.execPath + endoBin so it cannot bind to a global endo on PATH. - Remove the false "normalizer rejects an undefined numeric" comment (it defaults undefined) and use a non-mutating --origin collector. - Document that re-mk on an existing name rebinds without revoking. - Reconcile the design's placeholder-pending-namer contradiction (mk is the frozen Phase-1 spelling); add the Network group to the CLI reference the agent stack reads; lead the designs/README status with a taxonomy word and sync the totals; record the deferred #286 http-confine fix durably in the design doc. - Trim the changeset to consumer-facing facts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add the `endo http` subcommand tree with its first verb, `mk`, minting a
confined outbound-HTTP client capability under a host-curated policy:
endo http mk <name> --origin <url> [--origin <url>...]
[--max-requests-per-minute <n>] [--max-response-bytes <n>]
[--policy-mode strict|tofu-auto] [--as <agent>]
This is the stranded CLI surface from #286, rebuilt on the HTTP client that
already landed on `llm`. The daemon method is `provideHttpClient(name, policy)`
(backed by @endo/exo-http-client over @endo/http-confine), which mints one
client and holds its control facet host-side, so the verb takes a *policy* — an
origin allowlist plus optional rate/size/mode guards — rather than the
`http-controller`+`http-client` formula pair the original design assumed. The
daemon's normalizer is the authority on policy validity.
CLI-only; no daemon change. The design doc and its README entry are updated to
describe the verb sitting on the landed policy client rather than the formula
pair that no longer exists.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
process is already a built-in global in the eslint config (sibling CLI tests reference process.pid/process.env without a directive), so the declaration tripped no-redeclare and failed the lint check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apply the panel's must-fix items on PR #1014: - Document the --policy-mode tofu-auto widening on the minting surface itself (flag --help, changeset, design doc): it auto-allows any first-seen origin, so --origin only pre-seeds and the allowlist no longer confines outbound reach. Prefer strict until inspect/revoke land. - Validate flag lexical shape locally, reporting by flag name: --origin is normalized to its canonical WHATWG origin (accepting browser-copied trailing-slash / default-port forms), the numeric guards must be positive integers, and --policy-mode must be strict|tofu-auto. Semantic policy validity still defers to the daemon normalizer. - Fix the --origin metavar from <url> to <origin> and describe the exact shape the daemon accepts. - Extract a pure makeHttpClientPolicy assembler (packages/cli/src/http-mk-policy.js) and unit-test the record shape, omitted-when-unset knobs, origin order, normalization, and the numeric-flag parser — the policy is now observed, not just the echoed pet name. - Drop the dead live-HTTP-server fixture (nothing dialed it) and the inline import() JSDoc type; invoke the CLI under test via process.execPath + endoBin so it cannot bind to a global endo on PATH. - Remove the false "normalizer rejects an undefined numeric" comment (it defaults undefined) and use a non-mutating --origin collector. - Document that re-mk on an existing name rebinds without revoking. - Reconcile the design's placeholder-pending-namer contradiction (mk is the frozen Phase-1 spelling); add the Network group to the CLI reference the agent stack reads; lead the designs/README status with a taxonomy word and sync the totals; record the deferred #286 http-confine fix durably in the design doc. - Trim the changeset to consumer-facing facts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ec10200 to
4851b13
Compare
Panel review — round 2 — must-fixThe scripted code panel (single round) fanned its seats against Must-fix
Should-fix
The remaining seats (assessor, stylist, curator, breaker, engine-realist, Posted by the scripted panel (gauntlet stage: panel, round 2). Full per-seat model |
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>
The `refuses any path/query/fragment/userinfo suffix` property found a counterexample on CI (`http://host/..`): a path segment of only dot-segments (`.`/`..`) is collapsed back to the bare-origin root by URL path-normalization, so the input is a correct acceptance, not a reject case. Lead each generated suffix segment with a literal non-dot character so it can never be a lone dot-segment; the function's behavior is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriscendobot
left a comment
There was a problem hiding this comment.
Code panel — round 3 verdict: must-fix
Single-round code panel over origin/llm...HEAD (head 1f394cd03f1d1a2a4572291094d93fb9bb003af3). Per-seat verdict blocks follow; the round disposition is must-fix (at least one concrete must-fix finding stands). Address the must-fix findings, then the gauntlet re-reviews the delta.
assessor
Correctness / control-flow review — PR #1014 (endo http mk), diff base origin/llm
Verification performed (worktree …pr286-cli-verb-rework-gauntlet-panel-3, Node 22 local):
- All 18 non-daemon tests pass; the daemon-driven happy path passes end to end (2.7s) —
provideHttpClientexists where invoked andmkprints exactly the pet name. - Both new property arbitraries hold at 20 000 runs each (reject-partition and idempotence), so the
http://host/..counterexample fix in1f394cd03fclosed the flake rather than masking it. - CLI canonicalization matches the two downstream verbatim comparisons:
assertHttpClientOrigin'sparsed.origin !== origin(packages/daemon/src/host.js:186) and the exo'soriginOf(url)(packages/exo-http-client/src/http-client.js:238,:844). Documented defaults (60 rpm / 1 MiB /strict) matchnormalizeHttpClientPolicy(host.js:243-265). - Reproduced the local-reject paths: a bad
--max-response-bytesfails by flag name with no daemon connect;makeHttpClientPolicyruns beforewithEndoAgent, so an empty/invalid allowlist never opens a socket. designs/README.mdtotals arithmetic reconciles (43+23+40+31+2+7+2+3 = 151).
assessor
Verdict: approve
Findings:
- should-fix (doc-only): the two docs added here disagree on
--as.provideHttpClientis onHostInterfaceonly (packages/daemon/src/interfaces.js:369, insideHostInterface262-534) — neverGuestInterface— so--as <guest>fails with the exo's generic method-not-found after connecting. The changeset says--as <host>;designs/cli-http-client.mdsays[--as <agent>]andpackages/cli/src/commands/http-mk.js:33says "Agent to act as." Given the module's own stated principle (validate locally, report by flag name, don't forward a doomed call across CapTP), name host-only on the verb's help/JSDoc and pick one spelling. [proposed-rule: when a CLI verb's underlying daemon method exists on only one agent facet, the verb's own help text states that restriction, not only its changeset.] - comment-only:
normalizeHttpClientOriginaccepts hosts/ports that can never match at request time —https://example.com.normalizes tohttps://example.com.(≠ the request'shttps://example.com) andhttps://example.com:0is undialable. Both fail safe (a mint that reaches nothing), so no change requested; noting the accepted-but-inert corner. [rule: roles/jurors/assessor/AGENT.md § invariant-claim overlap]
Notes (out of scope but worth flagging):
- The coercers throw plain
Error, so a bad flag prints a SES stack dump rather than a cleanerror: option … is invalid. Commander 5.1.0 exports onlyCommanderError(noInvalidOptionArgumentError) and--deny/--allowalready behaves this way, so this is a package-wide clean-error pass, not this PR's. [proposed-rule: n/a — follow-up.] designs/cli-http-client.md's ownStatusis prose ("Phase 1 … landed on the policy client") while the README row now reads "In Progress"; every other design leads Status with a taxonomy word. Archivist's axis. [rule: designs/README.md status taxonomy]tofu-autois mintable with noinspect/revokeverb; disclosed on the flag help, changeset, and design. Accepted residual, correctly documented.
Self-improvement: my first pass leaned on reading the diff; the value came from executing it — running the suite, re-running the property arbitraries at 200× the default numRuns, and probing 20 hand-built origin edge cases against the real normalizer. Next engagement I will reach for the live probe before writing prose, and I will resolve the downstream comparison site (here the exo's originOf) before judging any CLI-side normalization claim, since "canonical" is only meaningful relative to the consumer that compares.
typist
Review complete — typist seat, PR #1014 (endojs/endo-but-for-bots)
Diff reviewed: origin/llm...HEAD (10 files; new packages/cli/src/http-mk-policy.js, src/commands/http-mk.js, endo.js wiring, tests, docs). I could not run tsc — this worktree has no installed node_modules — so findings are static.
typist
Verdict: request-changes
Findings:
packages/cli/src/http-mk-policy.js:165-171—makeHttpClientPolicy's@returnsdeclaresmaxRequestsPerMinute?/maxResponseBytes?/policyMode?optional, but that record is passed atsrc/commands/http-mk.js:48toprovideHttpClient(petName, policy: HttpClientPolicy), whose declared parameter (packages/daemon/src/types.d.ts:425) has all four fields required. Runtime is correct —normalizeHttpClientPolicy(packages/daemon/src/host.js:217) defaults each — but no declared type describes the input policy;HttpClientPolicyconflates caller-input with the normalized formula record, and this is its first external consumer. Fix: export an input alias (Partial<HttpClientPolicy> & { allowedOrigins: string[] }) from the daemon and@importit. must-fix-adjacent; invisible totsctoday becausecommands/http-mk.jshas no// @ts-checkunderpackages/cli/tsconfig.json'scheckJs: false. [rule: roles/jurors/typist/AGENT.md § Type-runtime drift]- The
'strict' | 'tofu-auto'union is hand-spelled at six sites (http-mk-policy.js:108,116,132,139,164,169,commands/http-mk.js:27) and duplicated as an untypedstring[]athttp-mk-policy.js:101— which is exactly why line 116 needs a cast — whileHttpClientPolicyModealready exists atpackages/daemon/src/types.d.ts:417. Two sources of truth; the design explicitly contemplates widening the mode set (tofu-prompt/tofu-attenuator), and the CLI copies would drift silently. Fix:@importthe mode and type the constreadonly HttpClientPolicyMode[]. Note both names are absent from the daemon's publicpackages/daemon/types.d.tsre-export list, so the fix includes adding them. [rule: roles/jurors/typist/AGENT.md § Secondary surface (public-API signature correctness)] designs/README.md:471— the added totals line uses→twice ("In Progress goes 22 → 23 and Proposed 32 → 31"); the preceding line in the same paragraph spells the identical relation39 -> 40. Fix: ASCII->. [rule: skills/typist-friendly-code-points/SKILL.md]packages/cli/src/commands/http-mk.js:29—@param {string} [args.agentNames] - Agent to act asomits the invariant the changeset states ("host-only, so a guest cannot mint one"):provideHttpClientis declared only inHostInterface(packages/daemon/src/interfaces.js:369), notGuestInterface. A guest pet name satisfies the declared type and fails at the interface guard. [proposed-rule: a JSDoc@paramwhose admissible values are narrower than its declared type because of a callee's interface-guard restriction names that restriction inline.]
Notes (out of scope but worth flagging):
commands/http-mk.jsomits// @ts-checkwhile its new siblinghttp-mk-policy.js:1opts in, so the args-record contract betweenhttpMkArgsFromOptsandhttpMkis unverified; adding it would catch finding 1 mechanically. Only 6 of 60src/files opt in today, so absence is the local norm.- Checked clean: the default claims (60 req/min, 1 MiB) match
host.js:246,257;parsePositiveIntegerFlag's documented rejection list matches its regex +isSafeIntegerpair;collectHttpOrigin'sprevioustyping matches commander's two-arg coercer;httpMkArgsFromOpts'sallowedOrigins: string[] | undefinedis consistently required-but-undefinable across all three hops.
Self-improvement: the recurring shape here — a CLI re-spelling a daemon type structurally rather than importing it, because the type is not in the package's public re-export list — is worth encoding as a typist standing rule ("when a diff hand-spells a type the callee package already declares, the finding includes whether the callee's public types.d.ts re-exports it"). Forwarding as a [proposed-rule] candidate to the gardener.
stylist
Reviewed the diff at origin/llm...HEAD (6 commits, 10 files).
stylist
Verdict: request-changes
Findings
1. httpMkArgsFromOpts and its opts parameter abbreviate two words each. (must-fix)
packages/cli/src/http-mk-policy.js:143, imported at packages/cli/src/endo.js:21 and called at :842. Args -> Arguments, Opts -> Options: httpMkArgumentsFromOptions = (name, options) => .... The cmd.opts() call is commander's own spelling and stays; the identifier and parameter we coin do not. The JSDoc at :127 already spells the type out as opts - cmd.opts() from the commander action, so the doc leans on the platform name to excuse ours. [rule: roles/jurors/stylist/AGENT.md § Abbreviated identifiers; skills/pre-push-gates/SKILL.md spell-out-identifiers]
2. Single-letter and abbreviated locals in freshly-authored code. (must-fix)
const n = Number(trimmed)atpackages/cli/src/http-mk-policy.js:79->parsedInteger.segatpackages/cli/test/http-mk-command.test.js:152and:153->segment. It contradictsarbSuffixSegmentdeclared eight lines above at:142, which spells the same word out.accattest:217->collectedOrigins;sattest:144->suffix;argsattest:259->mkArguments.
The arb* prefix itself is fine: packages/marshal/test/rankOrder.test.js:27 and packages/sha256/test/sha256-property.test.js:34 establish it as the repo's fast-check convention. dirname/endoEnv likewise mirror packages/cli/test/trace.test.js:8,11. [rule: as above]
3. parsePositiveIntegerFlag names a parser but is a factory, and its own docstring says so. (should-fix)
packages/cli/src/http-mk-policy.js:62 opens "Build a commander coercer", while :72 names it parse.... It sits in the option list at packages/cli/src/endo.js:826 and :831 beside parsePolicyModeFlag (:838), which really is the coercer its name claims, so two same-shaped names carry different arity. The package's factory convention is make*, used in this very file by makeHttpClientPolicy at :172. Propose makePositiveIntegerFlagParser. [rule: roles/jurors/stylist/AGENT.md § Secondary surface, name-and-docstring disagreement]
4. http-mk-policy.js is named for one of its five exports. (comment-only)
Four of the five (parsePositiveIntegerFlag, collectHttpOrigin, parsePolicyModeFlag, httpMkArgsFromOpts) are flag plumbing, not policy; only makeHttpClientPolicy and its helper normalizeHttpClientOrigin are. http-mk-flags.js, or a split, would stop the name understating the module.
5. mk as a bare verb has no in-package precedent. (comment-only, not blocking)
mkhost, mkguest, mkdir, mktmp are all mk+noun compounds carried over from unix; the package's bare standalone verb for the same idea is make (packages/cli/src/commands/make.js:18, endo make). endo http mk is the first bare mk. I am not asking for a rename: the "frozen Phase-1 spelling" note at designs/cli-http-client.md:92 was authored in this same PR, so it asserts its own precedent rather than citing a directive, and either spelling is defensible. Flagging so the namer dispatch the design defers to for allow/deny/revoke/inspect sees this one too. The exported httpMk and the file name commands/http-mk.js are correct either way: mirroring the shipped verb exactly is what mkhost.js:6 and mkdir.js:6 do.
No gratuitous renames found; the diff is additive. [rule: skills/rename-discipline/SKILL.md]
Self-improvement: the stylist brief's abbreviation rule needs a stated carve-out for a verb-mirroring identifier. I nearly flagged httpMk before checking mkhost.js/mkdir.js, where the export deliberately mirrors an abbreviated CLI verb; the rule as written reads as a flat must-fix, and the "established platform names" exemption covers os.tmpdir() but not "the surrounding package's own command-verb spelling". Routing that as a message to the liaison.
packager
Juror: packager — PR #1014
Verdict: request-changes
Findings
1. must-fix — the changeset's normalization claim has no test at the seam that implements it.
packages/cli/test/http-mk-command.test.js:75, titled makeHttpClientPolicy normalizes browser-copied origin forms, never calls makeHttpClientPolicy; its three assertions call normalizeHttpClientOrigin directly. Every other makeHttpClientPolicy test (lines 40, 50, 65) passes already-canonical origins, so nothing pins that the assembler applies the .map(normalizeHttpClientOrigin) on packages/cli/src/http-mk-policy.js:182. I verified by mutation: with that .map removed, all 20 non-daemon tests in the file still pass. The changeset asserts to consumers that "each value ... is normalized to its canonical serialization, so a browser-copied form with a trailing slash or an explicit default port is accepted" — the only path a user reaches is mk → makeHttpClientPolicy → provideHttpClient, and that path is unpinned. Fix: assert through the assembler, e.g. makeHttpClientPolicy({ allowedOrigins: ['https://Example.com/', 'https://example.com:443'] }) deep-equals { allowedOrigins: ['https://example.com', 'https://example.com'] }, and retitle or keep the direct-call test under a normalizeHttpClientOrigin title. [rule: skills/test-title-spec-spelling/SKILL.md] (Disclosure: the worktree carries that .map deletion as an uncommitted edit marked // MUTANT; it is not in the PR's commits and my diff review is of origin/llm...HEAD.)
2. should-fix — designs/README.md:471 breaks the newest-first totals order. The new 2026-08-17 line was inserted below the 2026-08-16 line, so a reader scanning top-down hits the superseded 08-16 line first, still asserting "This supersedes the counts in the historical summaries above." Totals themselves are internally consistent (sum 151, In Progress 22→23, Proposed 32→31). Move the new line above. [proposed-rule: an appended "Current totals" line leads the block, newest first, since the preceding line claims currency]
3. should-fix — design Status row diverges from the index. designs/cli-http-client.md:8 now reads Phase 1 (\endo http mk`) landed on the policy client, with no taxonomy word, while designs/README.mdsaysIn Progress. Round 2 applied the taxonomy-word fix to the README side only; the dominant convention across designs/*.md is taxonomy word first, qualifier in parens (In Progress (Phase 3)`).
4. should-fix — one-sentence-per-line broken by the round-2 inline insertions. designs/cli-http-client.md:64, :77, :80 leave ragged half-lines (while the daemon's, Because Phase 1) and mid-line sentence starts. [rule: AGENTS.md § Markdown style]
5. comment-only — stale metavar in the changeset synopsis. It writes [--as <host>]; the CLI help (endo.js:31) and the design synopsis both say --as <agent>. Same class as the <url>→<origin> fix taken in round 2; the prose sentence below it already carries the host-only semantics.
Clean on my axis
yarn.lock split into chore: Update yarn.lock (59168bcf7e) and ordered after the package.json commit 1517b8623c [rule: skills/yarn-lock-separate-commit/SKILL.md]; @fast-check/ava added as catalog:dev matching the root catalog, with a one-line lock delta. Bump minor is right for a new verb. @endo/cli is the only publishable package touched — @endo/lal is private: true, so the primer edit needs no changeset entry. No drive-by refactors or generated-file churn mixed into substance commits; review-response commits kept separate per skills/review-feedback-followup-commits/SKILL.md.
Self-improvement: the recurring packager finding this round was not conflated autofix but a test whose title names the function under test while its body exercises a collaborator, which lets the very behavior the changeset advertises go unpinned. Worth adding to my norms: when a changeset makes a transformation claim, locate the assertion that runs the user's call path, not a helper's, and mutate the transformation to confirm something fails.
archivist
archivist
Verdict: request-changes
Findings:
-
must-fix
packages/cli/src/commands/http-mk.js:12-14— the JSDoc says "The daemon'snormalizeHttpClientPolicyis the authority on policy validity; this verb assembles the record and lets a bad field surface as its structured error." The shipped verb does the opposite for lexical shape:makeHttpClientPolicy(src/http-mk-policy.js:160-175) throws on a missing/empty--origin, and the commander coercers reject a bad origin, non-positive-integer knob, or bad mode locally, before any CapTP call (pinned by the test attest/http-mk-command.test.js:377"Never reaches the daemon"). The module JSDoc ofhttp-mk-policy.js:3-8and the design both state the correct split (lexical local, semantics daemon); only this docstring still lies. Reword to "…is the authority on policy semantics; this verb validates each flag's lexical shape locally and reports by flag name." [rule: roles/jurors/archivist/AGENT.md § Operating norms — docstring-vs-code disagreement] -
should-fix
designs/cli-http-client.md:212-240—### \endo http` subcommand treestill documentsendo http mk <origins...>(positional origins, no--origin), saysmk"produces the controller / client pair under a single user-facing name", and line 228 says "(Verb names above are placeholder…)" — all three contradicted by the new## Landed CLI surface (Phase 1), which explicitly freezesmkand states there is one client name. The new section's consequences sentence (line 118) disclaims only "the mint signature, formula-type, andmakeHttpClient` sketches", so this section carries no marker; a reader landing on it reads a false CLI. [proposed-rule: a design section superseded by a newly added "Landed" section carries an inline pointer at its own heading, not only a blanket sentence naming other sections] -
should-fix
packages/cli/test/http-mk-command.test.js:38,211,267,296,330— five section banners (// --- Pure policy assembly (no daemon) ------…). Same decoration class the house rule forbids; keep the title, drop the rule run. The same-line title+rule shape slips the detector's "rules and nothing else" regex, so the seat is the only backstop. [rule: skills/no-comment-banners/SKILL.md] -
should-fix
.changeset/cli-http-mk-phase-1.md:12-16— says a "browser-copied form with a trailing slash or an explicit default port is accepted" but never says a path/query/fragment/userinfo form (https://api.example.com/v1) is refused. The prose invites pasting a browser URL; the refusal is the security-relevant half and is documented only in the design. Add one sentence. [proposed-rule: a changeset documenting input normalization states what is refused alongside what is accepted when the refusal is the security-relevant half]
Notes (out of scope but worth flagging):
.changeset/cli-http-mk-phase-1.md:8writes[--as <host>]while the CLI help anddesigns/cli-http-client.md:52both print--as <agent>; the accompanying prose ("names a host; the capability is host-only") is correct and verified againstpackages/daemon/src/interfaces.js:369(HostInterface only). Metavariable only. [rule: roles/jurors/archivist/AGENT.md § Operating norms]- Verified accurate, no action:
designs/README.md:471arithmetic (22→23 In Progress, 32→31 Proposed, 151 total), thedecide()tofu-auto citation (packages/exo-http-client/src/http-client.js:390), the 60/1 MiB defaults (packages/daemon/src/host.js:244,255), andredirect: 'manual'(packages/http-confine/src/http-confine.js:547).
Self-improvement: the recurring shape this round is the new section that supersedes an old one but disclaims only part of it — worth adding to the archivist brief as a standing check: when a diff adds a "Landed"/"As built" section to an existing design, enumerate every prior section describing the same surface and confirm each is either updated or explicitly marked, rather than trusting the new section's blanket consequences sentence.
prover
PR #1014 — endojs/endo-but-for-bots — juror: prover
Verdict: request-changes
Mutation-tested the diff in the worktree (baseline: 20/20 non-daemon tests green; daemon-driven mk exercised by hand against a live daemon).
Findings
-
must-fix —
makeHttpClientPolicy's origin normalization is pinned by no test. [rule:skills/regression-evidence/SKILL.md] I replacedpackages/cli/src/http-mk-policy.js:181(allowedOrigins.map(normalizeHttpClientOrigin)) with a pass-through of the raw values; all 20 tests still pass. The three tests that do callmakeHttpClientPolicy(test/http-mk-command.test.js:40,51,65) only feed already-canonical origins, and the daemon test (:319) passes canonical origins and asserts only the echoed pet name plusendo list. The same hole hides thecollectHttpOriginwiring inendo.js:820(a last-wins regression there leavesmk --origin a --origin bwith one origin — nothing asserts what reached the daemon) and the--max-*option-key wiring (unit-pinned only athttpMkArgsFromOpts, not through commander). This is the security-relevant path the docstring itself argues for ("would teach a false confinement").
Verified fix: the existing serial daemon test can assert it end to end —endo inspect <name>renders the stored policy. Runningendo http mk my-http --origin 'https://Example.com/' --origin 'https://api.example.com:443' --max-response-bytes 2048thenendo inspect my-httpprintsallowedOrigins: [https://example.com, https://api.example.com]/maxResponseBytes: 2048/maxRequestsPerMinute: 60/policyMode: strict. That single assertion reddens on all three mutants (the daemon rejects a rawhttps://Example.com/, sinceassertHttpClientOrigincomparesnew URL(o).origin === o). -
should-fix —
parsePositiveIntegerFlag's safe-integer guard is dead coverage. [rule:skills/coverage-driven-testing/SKILL.md] Deletinghttp-mk-policy.js:75-81(Number.isSafeInteger) leaves the suite green — verified. The regex at:70admits9007199254740993, whichNumber()mangles to…992.test/http-mk-command.test.js:195tries only lexical rejects. Addt.throws(() => parse('9007199254740993'), { message: /safe integer/ }). -
comment-only — test title names a function it never calls. [rule:
skills/test-title-spec-spelling/SKILL.md]test/http-mk-command.test.js:74is titled "makeHttpClientPolicy normalizes browser-copied origin forms" but every assertion callsnormalizeHttpClientOrigindirectly. Route it throughmakeHttpClientPolicy(which also discharges part of finding 1) or retitle.
Self-improvement: the surviving mutant here was invisible to reading — every test named the normalizer, so the suite looked load-bearing. Cheap generalization: when a pure helper is composed into an assembler, mutate the composition site, not the helper; fixture data that is already in canonical form is the tell.
curator
Review complete — juror block below
curator (public API surface / exported identifier shape)
Verdict: request-changes
Findings:
-
packages/cli/src/http-mk-policy.js:101,108,116,132,139,164,169— the HTTP-client policy shape and its'strict' | 'tofu-auto'mode union are declared independently in the CLI, whilepackages/daemon/src/types.d.ts:417,425already declares canonicalHttpClientPolicyMode/HttpClientPolicy. The CLI re-spells the union in six JSDoc positions plus a runtimeHTTP_POLICY_MODESarray (and duplicates the daemon'sHTTP_ORIGIN_SCHEMES,packages/daemon/src/host.js:163, at:11). Drift here is not hypothetical:host.js:207-213namestofu-prompt/tofu-attenuatoras modes a later phase wires in, and when the daemon widensHTTP_CLIENT_POLICY_MODESthe CLI's private copy keeps rejecting them by flag name with no type error. The CLI cannot import the canonical name today —packages/daemon/types.d.tsre-exports 30-odd types but neitherHttpClientPolicynorHttpClientPolicyMode, and daemonexportshas only".", so a deep@importfromsrc/types.jsis unresolvable. Fix: add both to the daemon's exported type list, then/** @import { HttpClientPolicyMode } from '@endo/daemon' */per the existing convention atpackages/cli/src/render-retention-path.js:3andcommands/paths.js:7, and add a'@endo/daemon': patchline to the changeset. should-fix. [rule: roles/jurors/curator/AGENT.md § Operating norms — cross-package option types live in one canonical package and re-export] -
packages/daemon/src/types.d.ts:1629declaresprovideHttpClient(petName, policy: HttpClientPolicy)with all four fields required, but the accepted surface is the partial record this PR sends.normalizeHttpClientPolicytreats every field as optional (host.js:236-266), andendo http mk's documented contract — omit the guard knobs so the daemon applies 60/min and 1 MiB — depends on that. A typed SDK caller replicatingmakeHttpClientPolicy's output would fail typecheck; the CLI escapes only becausepackages/cli/tsconfig.jsonsetscheckJs: falseandcommands/http-mk.jshas no// @ts-check. Export a distinct input type and use it in the signature. comment-only. [proposed-rule: when a normalizer accepts a partial record, the exported method signature must name the input type, not the normalized one]
Notes (out of scope but worth flagging):
- Bump level is correct: a new verb on
@endo/cliasminormatches.changeset/cli-mount-denied-segments.md.@endo/cliisprivatewith"exports": {}, so the six newhttp-mk-policy.jsexports are test-visible only — the surface delta is CLI-verb-only, and the changeset names the one package the PR touches. [rule: skills/changeset-discipline/SKILL.md] --originstays required under--policy-mode tofu-auto, where per the design it pre-seeds rather than bounds; the flag's arity is mode-invariant while its meaning is not. Documented in the changeset and--policy-modehelp, so no change asked. [proposed-rule: a flag whose confinement meaning inverts by mode should say so on its own help line — already satisfied here]
Self-improvement: the recurring curator catch on this repo is not renamed exports but unexported ones — a canonical type declared in src/types.d.ts and omitted from the package's public types.d.ts re-export list, forcing every downstream consumer to re-declare it. ShellPolicy (types.d.ts:367) has the same gap. Worth checking the re-export list against src/types.d.ts whenever a PR is the first cross-package consumer of a daemon-owned record.
migrator
Reviewed origin/llm...HEAD (6 commits, 10 files) in the named worktree.
Juror: migrator — PR #1014
Verdict: request-changes (one should-fix; no must-fix)
Bump level itself is right: @endo/cli: minor for a purely additive verb tree (endo http/mk), no exported identifier renamed, no prior caller's contract moved. provideHttpClient already exists on origin/llm (packages/daemon/src/host.js:927, packages/daemon/src/interfaces.js:369), so no coordinated @endo/daemon bump is required for the call to resolve. The @fast-check/ava devDep rides the existing catalog:dev entry (.yarnrc.yml:3, ^3.0.1), whose ava peer (^7||^8) is satisfied by the catalog's ^8.0.1, and matches the pattern in six sibling packages; yarn.lock gains only the workspace descriptor line, in its own chore: commit.
Findings
1. @endo/lal is missing from the changeset (should-fix).
packages/lal/primer/cli-reference.md is shipped content — primer is in packages/lal/package.json files, and packages/lal/prompts/system.js:76 instructs the agent to readText("primer", "cli-reference.md") at runtime. Editing it changes @endo/lal's published behavior with no version signal. Direct in-repo precedent: 3486b438b7 edited packages/lal/primer/tools.md and carried '@endo/lal': minor in .changeset/lal-fs-search-parity.md. Add '@endo/lal': patch to .changeset/cli-http-mk-phase-1.md (or a sibling changeset). [rule: skills/changeset-discipline/SKILL.md]
2. The daemon's policy contract is hand-copied into the CLI with no type link (comment-only).
packages/cli/src/http-mk-policy.js:81 re-declares HTTP_POLICY_MODES = ['strict','tofu-auto'] and :12 re-declares HTTP_ORIGIN_SCHEMES, duplicating packages/daemon/src/host.js:162-163; the mode is typed as a local literal union rather than HttpClientPolicyMode (packages/daemon/src/types.d.ts:417), which is not re-exported from packages/daemon/types.d.ts. When the daemon widens to tofu-prompt/tofu-attenuator — already named as deliberately-deferred in host.js's normalizer docstring — the CLI coercer silently rejects a mode the daemon accepts, and lint:types will not catch it. This repo already solved exactly this shape once: .changeset/daemon-type-guards-export.md exports the daemon's shapes "so consumers … without redefining them locally" (@endo/daemon: minor + consumer patch). Same remedy applies here, as a follow-up. [rule: .changeset/daemon-type-guards-export.md precedent]
3. Daemon-owned defaults are quoted in the CLI changelog (comment-only).
The changeset asserts "60 requests/minute and a 1 MiB response cap"; those live in normalizeHttpClientPolicy (packages/daemon/src/host.js:240,256). A published CHANGELOG entry is immutable, so a later daemon default change leaves @endo/cli's history wrong. Prefer "the daemon's defaults" over the literals. [proposed-rule: a changeset states only its own package's contract; cross-package defaults are referenced, not transcribed]
Self-improvement: checking the shipped-files manifest (package.json files) before deciding a doc edit is changeset-free turned an easy pass into finding 1; folding that into the migrator's default sweep.
locksmith
Juror: locksmith — PR #1014
Verdict: request-changes
The flag→policy assembly is careful capability work: origins are canonicalized to the exact form assertHttpClientOrigin compares verbatim, and a path/query/fragment/userinfo suffix is refused rather than silently widened to the whole host (packages/cli/src/http-mk-policy.js:32-64) — the right call on a minting verb, and property-tested. My findings are about the authority the verb can hand out, not the parsing.
Findings
-
must-fix —
--policy-mode tofu-automints an unbounded, unrevocable, unauditable outbound capability, gated only by prose.packages/cli/src/endo.js:834.decide()inpackages/exo-http-client/src/http-client.js:390returnsallowfor any unbound target under this mode, so--originstops confining and the client reaches arbitrary origins (redirects too —resolveRedirectchecks the same growing set). Phase 1 ships noinspect/revokeverb, which the PR's own changeset concedes ("Preferstrictuntil the policy-inspection and revocation verbs land"). A capability whose own documentation says don't use it yet should be structurally absent, not denied at runtime by a help string — the classicif (readOnly)shape [rule:roles/jurors/locksmith/AGENT.md§ runtime-flag attenuation]. Minimal fix: drop'tofu-auto'fromHTTP_POLICY_MODES(http-mk-policy.js:97) until the phase-2 verbs land, or require an explicit second opt-in flag. -
should-fix — re-
mkorphans the control facet, not merely "does not revoke".packages/cli/src/commands/http-mk.js:48. The control facet is WeakMap-keyed on the client cap and reached only viagetHttpClientControl(clientCap)(packages/daemon/src/host.js:948); rebinding the pet name removes the only CLI-addressable handle to the old client, so the surviving capability becomes permanently unrevocable — worse than the docs' "not revoked". Makemkrefuse an occupied name absent--forcerather than warning in three prose locations. -
should-fix — the loopback/link-local reach is unremarked on the mint path. The daemon's own test mints
--origin http://127.0.0.1:8080; nothing inhttp-confine/exo-http-clientscreens loopback, link-local, or private literals. A client handed to a guest therefore reaches the daemon's ownENDO_ADDRlistener andhttp://169.254.169.254. Ask: warn on stderr (or--allow-private-network) when an entry is a private/loopback/link-local literal. -
should-fix — the host-only bound is asserted but untested at the CLI seam. The changeset claims "a guest cannot mint one"; no test drives
mk --as <guest>. Pin it, so a later interface-guard move can't silently make outbound-network minting guest-reachable [rule:skills/adversarial-tests/SKILL.md]. -
comment-only — the guard knobs only ratchet outward.
parsePositiveIntegerFlag(http-mk-policy.js:72) has no ceiling, so--max-response-bytes 9007199254740991disables the 1 MiB cap while reading as "Cap each response body".mkalso echoes only the name (http-mk.js:49), never the effective policy, and with noinspectverb the operator can never see what was confined. [proposed-rule: a CLI verb that mints a capability echoes the effective policy it minted, including daemon-applied defaults.]
Self-improvement: the sharpest finding here came from following the attenuator to its revocation handle, not just its allowlist — "who can undo this grant, and through what name" is a question I should ask on every provide* front-end, and I've noted the WeakMap-keyed-control-facet pattern as the shape where a name rebind silently ends revocability.
warden
Per-juror block — warden — #1014
Verdict: approve (one should-fix; no must-fix)
Reviewed git diff origin/llm...HEAD in the panel-3 worktree: new endo http mk verb (packages/cli/src/commands/http-mk.js, packages/cli/src/http-mk-policy.js), its endo.js wiring, tests, changeset, and docs. Docs and changeset are clean of the recurring warden hazard — no globalThis.X initializer, no Object.prototype sample, no prototype walk in any snippet — and the tofu-auto widening is stated on the minting surface itself (--help) rather than only in the design doc, which is the right place for a confinement-nullifying switch.
Findings
1. should-fix — packages/cli/src/http-mk-policy.js:11,101: the mirrored allowlist constants drop the harden the mirrored code applies.
HTTP_ORIGIN_SCHEMES (:11) and HTTP_POLICY_MODES (:101) are the two arrays this module consults to make its only security decisions — scheme admissibility at :37 and policy-mode admissibility at :111. They mirror packages/daemon/src/host.js:162-163 name-for-name and value-for-value, where both are harden(['strict','tofu-auto']) / harden(['http:','https:']). Under @endo/init lockdown the intrinsics are frozen, but a module-level array literal is not: any code in the CLI realm can HTTP_POLICY_MODES.push('tofu-prompt') and widen the mode gate, or splice a scheme into the origin gate. Blast radius is small (short-lived single-realm process, no confined guest code), so this is discipline, not a live exploit — but when you copy a hardened constant, copy the harden. [proposed-rule: a module-level constant consulted in an allow/deny decision is hardened at its definition site]
2. comment-only — packages/cli/src/http-mk-policy.js:184: policy record returned unhardened across the CapTP boundary.
makeHttpClientPolicy returns a fresh mutable record that http-mk.js:48 hands to E(agent).provideHttpClient. Not a defect: packages/captp/src/captp.js:549 hardens the argument array transitively before serialize, so it is frozen before it crosses. Noting it only because the CLI is inconsistent here — commands/form.js:26 hardens its record at assembly, mktmp.js/mkhost.js do not — and for a policy record on a capability-minting path the local harden is the cheap self-documenting guarantee. [rule: roles/jurors/warden/AGENT.md § Secondary surface]
3. comment-only — packages/cli/src/commands/http-mk.js:49: mint echoes the name, never the bounds.
The verb prints only the pet name, while normalizeHttpClientOrigin silently rewrites the host it was given (IDN → punycode, case-fold, default-port strip). The module's own docstring justifies the path/query/userinfo refusal on the grounds that the entry should mean "exactly what the user typed" — the same reasoning argues for echoing the effective allowedOrigins, so an operator minting a confined capability can confirm the confinement without an inspect verb (which Phase 1 does not ship, as the design doc notes). [proposed-rule: a capability-minting verb echoes the effective bounds, not just the bound name]
Self-improvement: the decisive check this pass was empirically confirming that @endo/captp hardens outbound arguments before serialize (a one-line node probe against @endo/marshal plus grep -n harden packages/captp/src/captp.js), which demoted what looked like a must-fix "unhardened record crosses the boundary" into an accurate comment-only. Verify the transport's own harden behavior before scoring an unhardened-at-the-boundary finding as must-fix.
saboteur
Saboteur — PR #1014 (endo http mk), diff vs origin/llm
Verdict: request-changes
Attacks walked: 14 input shapes against normalizeHttpClientOrigin / parsePositiveIntegerFlag / parsePolicyModeFlag / makeHttpClientPolicy, run against the built CLI. Split: 3 real, 9 mitigated, 2 out of scope.
Real
1. Secret echo — --origin error prints userinfo credentials verbatim. packages/cli/src/http-mk-policy.js:52-58. Verified:
$ endo http mk x --origin 'https://alice:s3cr3t-token@api.example.com'
Error#1: --origin "https://alice:s3cr3t-token@api.example.com" must be a bare origin ...
The password (and any ?api_key=… on the query branch) lands in stderr. In this repo stderr is routinely an LLM agent's transcript, not just a human terminal, so the blast radius is wider than shell history. The guard is right to reject; only the echo is wrong. Fix: report the redacted parsed.origin plus the offending component name, never JSON.stringify(raw), once username/password/search is non-empty. Should-fix. [proposed-rule: an error message that echoes user input must redact credential-bearing components (URL userinfo, query) — echo the parsed, stripped form and name the component instead]
2. Flag-validation errors escape as a stack dump, and the tests cannot tell. Commander 5 has no InvalidArgumentError, so a plain Error from a coercer rethrows past main's isTerminalError filter into bin/endo.cjs's console.error(error). Verified contrast at the same exit code 1:
- new:
(Error#1)+ 4 stack frames +Error#1: --max-response-bytes must be a positive integer, got "0" - existing:
error: unknown option '--bogus'
This defeats the module docstring's own goal ("the user sees the flag they typed"). packages/cli/test/http-mk-command.test.js:298,313 assert only regex(stderr, /--max-response-bytes/), which passes under both surfaces — the criterion is unpinned. Same path swallows --as <guest> (host-only method) into a CapTP stack dump. Should-fix. [proposed-rule: CLI flag-validation failures must print a single error: line and exit non-zero; a test asserting the message must also assert no stack frame]
3. The idempotence property is tautological; the load-bearing invariant is unpinned. test/http-mk-command.test.js:112-120 — output is parsed.origin, so normalize(normalize(x)) === normalize(x) cannot fail. The cross-layer contract is the daemon's assertHttpClientOrigin (host.js:186): new URL(out).origin === out and the scheme is http(s). Assert that over arbAcceptedOrigin so a refactor returning anything but parsed.origin fails locally instead of after a CapTP round trip. [rule: skills/regression-evidence/SKILL.md]
Mitigated (no change)
Authority confusion (https://evil\@good, https://a.com:80@evil.com, #@), 2^53 boundary (9007199254740993 → rejected), 0x10/1e3/1_000, collectHttpOrigin accumulation, --origin-omitted, ftp:, blob:. Tight-try discipline: http-mk-policy.js:44-50 is exactly the narrow shape.
Out of scope
Dot-segment carve-out — --origin https://api.example.com/.. and /%2e%2e are accepted (URL path normalization collapses to /), so "no path" is not total. Same-host, so no widening; the test comment documents it but the module docstring does not. Loopback/link-local origins (169.254.169.254) mint fine — host-curated by design.
Breaker hand-off: whether tofu-auto plus no inspect/revoke verb breaks the "allowlist confines outbound reach" invariant is an invariant question, not an input one.
breaker
breaker
Verdict: request-changes
Findings:
-
packages/cli/src/commands/http-mk.js:16-19,.changeset/cli-http-mk-phase-1.md:31,designs/cli-http-client.md:87all assert the same invariant: re-mkon an existing name "does not revoke the previous one." The daemon falsifies it.packages/daemon/src/store-controller.js:59callsremoveEdgeIfUnreferenced(previousId)after the overwrite; that reachesgraph.js:190removeGroupEdge, whose refcount drop firesmaybeCollectand collects the orphanedhttp-clientformula immediately. Attack:endo http mk feed --origin https://a.example, thenendo http mk feed --origin https://b.example; the first client is collected, not retained. The claim is wrong in the direction that harms an operator, because "prefer a fresh name" steers them away from the one action that actually reclaims a too-wide client, so they accumulate live wide clients. Accurate wording: the rebind drops the name's reference, and the old client is collected unless another edge still retains it (for example it was granted to a guest). Must-fix in all three places, and pin it with a test alongsidepackages/cli/test/formula-collection.test.js. [proposed-rule: a documented claim about whether a capability survives a pet-name rebind must be pinned by a test, because the daemon's formula graph, not the CLI, decides] -
packages/cli/src/http-mk-policy.js:19states the module "deliberately does NOT silently drop" a suffix, because silent widening would "teach a false confinement." It nonetheless silently rewrites the host:https://ехample.com(Cyrillic е) normalizes tohttps://xn--ample-ywe6i.com, andhttp://127.1tohttp://127.0.0.1.mkprints only the pet name, so the operator never sees the allowlist that was minted. Echo the normalized origins on success. Same principle the module already argues. [proposed-rule: a capability-minting verb that canonicalizes a confinement bound must echo the canonical form it minted] -
--policy-mode tofu-automints a client with no bound (packages/exo-http-client/src/http-client.js:390,decide()returns allow for any unlisted target), noinspect, and norevoke, yetmakeHttpClientPolicystill requires--origin, manufacturing a bound that does not exist. Help text is not the mint-time surface. Warn on stderr at mint, or gate the mode behind an explicit acknowledgment flag until phase 2. [proposed-rule: a flag that voids the confinement the verb advertises must warn at mint time, not only in--help]
Notes (out of scope but worth flagging):
- Coercers throwing plain
Erroryields(Error#1)plus a stack trace rather than commander's cleanInvalidArgumentErrorline, but this matches house precedent (packages/cli/src/number-parse.jsparseBigint). No change asked. [rule: skills/panel-review/SKILL.md § Disposition rubric] https://example.com.is accepted and round-trips, but never matches a request tohttps://example.com. A dead allowlist entry, not a widening.
Self-improvement: the strongest breaker findings on a CLI front-end come from following a documented claim across the CapTP boundary into the daemon's storage and GC layers, not from attacking the front-end's own validators, which the author had already hardened.
purist
Journal result entry posted (entries/2026/08/17/101335Z-result-gardener-e8a682.md).
purist
Verdict: comment-only
Findings:
-
packages/cli/src/http-mk-policy.js:101duplicates the daemon's mode vocabulary (HTTP_CLIENT_POLICY_MODES,packages/daemon/src/host.js:162) and respells its union'strict' | 'tofu-auto'seven times (lines 108, 116, 132, 139, 164, 169) althoughpackages/daemon/src/types.d.ts:417already names itHttpClientPolicyMode. The daemon's docstring saystofu-prompt/tofu-attenuatorare excluded only until a livepolicyAuthorityis wired, so the copy is drift-prone: once the daemon widens, this coercer still rejects the new mode locally with "must be strict or tofu-auto" before the authority ever sees it. Fix: addHttpClientPolicyModeto the re-export block atpackages/daemon/types.d.ts:35and annotate withimport('@endo/daemon').HttpClientPolicyMode. should-fix. [rule: roles/jurors/purist/AGENT.md § Family-consistency across related symbols] -
packages/cli/src/http-mk-policy.js:178requires at least one--originunconditionally, while the authority it defers to (normalizeHttpClientPolicy,packages/daemon/src/host.js:227) treatsallowedOriginsas optional. Under--policy-mode tofu-autothe allowlist is a pre-seed and not a bound (the exo auto-allows any first-seen origin), so the verb demands an entry that confines nothing and the mint reads as origin-confined when it is not. That is the same false-confinement hazard the module's own docstring at lines 12-25 refuses for path-bearing origins, applied to the mode axis. Make the requirement mode-aware: require instrict, and undertofu-autoeither accept its absence or say on stderr that the allowlist does not bound outbound reach. should-fix. [rule: roles/jurors/purist/AGENT.md § Edge-case enumeration on values] -
packages/cli/src/http-mk-policy.js:11and:101are unhardened module-level arrays; the daemon counterparts atpackages/daemon/src/host.js:162-163areharden(...). Not a boundary defect (single realm, and CLI siblings do not harden), so consistency with the family it mirrors is the only argument. comment-only. [rule: roles/jurors/purist/AGENT.md § Property hygiene on frozen prototypes] -
packages/cli/src/http-mk-policy.js:20claims the refusal "keeps the allowlist entry meaning exactly what the user typed", but the same function lowercases the host and strips an explicit default port. Narrow the sentence to the path/query/fragment/userinfo case it actually defends. comment-only. [rule: skills/gricean-maxims/SKILL.md]
Notes (out of scope but worth flagging):
- Passability checked and clean: the record built at
http-mk-policy.js:184and sent atcommands/http-mk.js:48is unfrozen, but CapTP hardens arguments at send (packages/captp/src/captp.js:532) and this matchescommands/mount.js:32. Recorded so a later seat does not re-raise it. JSON.stringifyfor quoting user tokens in errors is the majority CLI convention (commands/define.js:19,form.js:19,endow.js:20,inspect.js:24); onlypet-name.jsusesqfrom@endo/errors. The@endo/*-reuse rule does not bite here.
Self-improvement: the reuse-an-@endo/*-primitive axis in roles/jurors/purist/AGENT.md reads as unconditional, but q-versus-JSON.stringify in packages/cli/src shows the local family can already have settled the other way; the axis should say to establish the surrounding package's spelling before flagging.
spec-keeper
spec-keeper
Verdict: approve (two cheap should-fixes worth landing in this PR; no must-fix)
The implementation is spec-correct where it counts. normalizeHttpClientOrigin emits new URL(x).origin, which is exactly what the daemon's assertHttpClientOrigin compares verbatim (packages/daemon/src/host.js:175-194) and what the exo computes per-request (packages/http-confine/src/http-confine.js:186) — the three normalizers agree. --policy-mode tofu-auto's help text ("AUTO-ALLOWS any first-seen origin") matches the authority: decide() returns allow unconditionally for that mode (packages/exo-http-client/src/http-client.js:390), so the widening claim is neither overstated nor understated. Refusing path/query/fragment/userinfo rather than letting .origin silently widen to the whole host is the right call on a capability-minting verb. No primordial-preservation findings: @endo/init locks down at packages/cli/src/endo.js:5 before any coercer runs, and the arrays involved are module-local.
1. should-fix — the idempotence property is spec-tautological over its arbitrary. test/http-mk-command.test.js:112-131. arbAcceptedOrigin draws hosts only from fc.domain() (lowercase registrable domains) + optional port + optional slash. Over that space normalize(normalize(x)) === normalize(x) cannot fail: the origin/host serializer re-parses to itself. The host forms where WHATWG rewriting is non-trivial — the only ones where a fixed-point or CLI↔daemon disagreement is conceivable — are all excluded, and unpinned anywhere: IPv4 parser shorthand/radix (http://127.1, http://0x7f.0.0.1 → http://127.0.0.1), IPv6 compression (http://[0:0:0:0:0:0:0:1]:80 → http://[::1]), percent-decoded host (http://exa%6dple.com → http://example.com), IDNA/ToASCII (https://例え.jp → https://xn--r8jz45g.jp). All four verified on Node 22. Add them as value assertions, widen the arbitrary's host, and name the property as the daemon's predicate (new URL(o).origin === o) so a future divergence is what breaks. [rule: skills/regression-evidence/SKILL.md; skills/adversarial-tests/SKILL.md § engine-variance]
2. should-fix — the Number.isSafeInteger guard has zero coverage. src/http-mk-policy.js:80; test :202. Every bad input listed fails at the regex, so that branch could be deleted with no test failing — yet it is the branch encoding the spec claim (Number.isSafeInteger + binary64 rounding). Verified: '9007199254740991' accepted, '9007199254740993' → 2^53 → rejected, a 400-digit numeral → Infinity → rejected. Add both boundaries and ' 1024 ' for the trim() path. [rule: skills/regression-evidence/SKILL.md]
3. comment-only — src/http-mk-policy.js:47: parsed.pathname !== '' is dead for a special scheme (path is never empty; the scheme check above guarantees http/https). Relatedly, https://example.com/# and /? are accepted (empty fragment/query serialize to '') — lossless, but worth a docstring word so the gate isn't read as "rejects any #".
4. comment-only — test :174: AVA's t.throws returns undefined on failure, which fast-check reads as a pass, so a real regression never shrinks and reports no minimal counterexample. return t.throws(...) !== undefined restores shrinking. The t.is at :130 returns a boolean and is fine.
Self-improvement: the highest-yield spec-keeper move on a URL-normalization diff is to run the candidate edge inputs through the actual engine before writing (host-parser rewrites are where new URL surprises), and to check whether a property test's arbitrary can even reach the branch its docstring claims to pin — a tautological property reads as coverage but pins nothing.
wire-watcher
wire-watcher — PR #1014
Verdict: comment-only
The load-bearing claim of this diff is that the CLI's local origin canonicalization agrees with the daemon's verbatim predicate. I checked it three ways — normalizeHttpClientOrigin (packages/cli/src/http-mk-policy.js:32) → assertHttpClientOrigin's parsed.origin !== origin (packages/daemon/src/host.js:184) → parseAllowedOrigins/checkOriginAllowed (packages/http-confine/src/http-confine.js:169,185) — and ran the canonicalizing cases (IDN, IPv6 long form, IPv4 shorthand, %2E host bytes, mixed case, :0443, trailing ?/#, \-as-/, /..). Every accepted input is a fixed point the daemon accepts; every widening input (path/query/fragment/userinfo) is refused locally. No parser divergence, no load-then-verify inversion.
Findings
-
should-fix — the unsafe-integer guard is dead in test.
packages/cli/src/http-mk-policy.js:80is unreachable from the reject list atpackages/cli/test/http-mk-command.test.js:202:abc/``/0/`-5`/`1.5`/`0x10`/`1e3`/`1_000` are all caught by the regex on line 74. The syntactically-valid-but-almost-certainly-wrong case is the one that matters — `'9007199254740993'` (2^53+1) reaches line 80 and is rejected by flag name. The daemon has the mirror test (`packages/daemon/test/http-client.test.js:137`); the CLI's whole claim is that it reports before the CapTP round trip, and only a test at this layer pins that. [rule: `skills/adversarial-tests/SKILL.md` — failure-mode catalog] -
should-fix — rebind orphans a live outbound-network capability past revocation.
provideHttpClientstores over the pet name (packages/daemon/src/host.js:938); the old client's control facet is reachable only viagetHttpClientControl(clientCap), and the cap is no longer nameable once the name is overwritten. Phase 1 ships no revoke verb, so a re-mkintended to narrow confinement silently leaves the wider client live and unrevocable. Prose in three places (docstring, changeset, design) is the only guard. Minimum: havehttpMkdetect an existing binding and warn on stderr, or gate rebind behind--rebind. The design's "matches the siblingprovide*mints" parity is weaker here — no sibling mint hands out outbound network reach. [rule: wire-watcher brief § protocol state-machine invariants (reachability)] -
should-fix — the
tofu-automarker is invisible at the moment it takes effect. Help text, changeset, and design all name the widening (good round-2 work), butpackages/cli/src/commands/http-mk.js:49prints only the pet name: atofu-automint is byte-identical on stdout to a strict one. Compounding it,makeHttpClientPolicystill requires--origin(line 178) in the mode where the list does not bound — the required flag itself teaches false confinement. One stderr line at mint time ("allowlist does not bound outbound reach; no revoke verb yet") puts thealg:none-shaped marker on the surface where it is used. [rule: wire-watcher brief § in-band-marker trust-bypass] -
comment-only — the agreement property is pinned over too narrow a space.
arbAcceptedOrigin(packages/cli/test/http-mk-command.test.js:112) draws hosts fromfc.domain()only. The idempotence property isassertHttpClientOrigin's predicate in disguise, so the cases worth pinning are exactly the ones where WHATWG canonicalization does work: IPv6 literals, IPv4 shorthand, a case-mangled host, an IDN host. All are fixed points today — this is hardening against a future normalization change, not a live bug. -
comment-only — the operator never sees the origin actually baked in.
--origin http://2130706433mints forhttp://127.0.0.1; a Cyrillic-аhomograph mints for its punycode. Echoing the normalized allowlist alongside the pet name would make the confinement the operator got legible at mint time. [proposed-rule: a capability-minting verb echoes the canonicalized bound it minted, not only the name it bound]
Self-improvement: this engagement's leverage came from executing the normalizer against the downstream predicate rather than reading both and asserting agreement — the fixed-point/verbatim-compare seam is cheap to run and the run is what let me clear parser-divergence outright and spend the block on the reachability and marker-visibility findings instead. Worth making the seat's default first move on any diff that normalizes a value one layer compares verbatim.
engine-realist
engine-realist
Verdict: request-changes
Findings:
-
packages/cli/src/commands/http-mk.js:49prints only the pet name, never the allowlist it actually minted.URLhost canonicalization is engine-defined and silently rewriting:--origin https://пример.examplemintshttps://xn--e1afmkfd.example,--origin http://0x7f.1mintshttp://127.0.0.1,--origin https://ex%41mple.commintshttps://example.com(all verified on Node 22). The module's own docstring (http-mk-policy.js:44) argues that quietly widening a path would "teach a false confinement"; the same argument applies to a host rewrite on a capability-minting verb, and here the rewrite is performed by whichever engine ran the CLI while the string it produces is persisted verbatim into a formula the exo re-parses at every incarnation. Echo the normalizedallowedOriginsafterprovideHttpClientresolves. [proposed-rule: a capability-minting CLI verb echoes the normalized bounds it minted, not just the name it bound.] -
packages/cli/src/http-mk-policy.js:58accepts a trailing-dot FQDN and passes it through unchanged:new URL('https://example.com.').originis'https://example.com.', which is a distinct string fromhttps://example.com, andcheckOriginAllowed(packages/http-confine/src/http-confine.js:186) matches bySet.hason the exact serialization. The result is a client that denies every request with anOriginNotAllowedErrornaming an origin visually identical to the allowed one. It fails closed, so it is not a hole, but it is a dead capability minted without complaint. Reject it by flag name, or warn; do not strip (the root label is a real origin distinction). [proposed-rule: reject host forms whose canonical origin cannot match the host form a user will actually dial.] -
packages/cli/test/http-mk-command.test.js:112-131draws hosts fromfc.domain(), which is exactly the region whereURLcanonicalization is not engine-defined (plain ASCII LDH labels, already fixed points). Every transforming host form is unpinned: IPv4 shorthand, IPv6 bracket collapse (http://[0:0:0:0:0:0:0:1]:8080becomeshttp://[::1]:8080), percent-encoded host octets, IDNA, trailing dot. The daemon-driven test useshttp://127.0.0.1:8080, itself a fixed point. Since the whole design rests on the CLI's serialization equalling the daemon's verbatimparsed.origin !== origincomparison (packages/daemon/src/host.js:186), pin the fixed point over those forms too. [rule: skills/regression-evidence/SKILL.md]
Notes (out of scope but worth flagging):
-
packages/cli/src/http-mk-policy.js:47:parsed.pathname !== ''is unreachable. For the two special schemes this function admits, a parsed URL always has a non-empty path. Harmless, but it suggests the empty case occurs on some engine. [proposed-rule: no dead engine-defensive branch without a named engine that needs it.] -
makeHttpClientPolicyrequires at least one--origineven under--policy-mode tofu-auto, where the daemon accepts an empty seed list. Defensible as deliberate friction; worth stating in the flag help if so. [rule: skills/panel-review/SKILL.md]
Self-improvement: added to the seat's watch list that "the engine" in a Node-only CLI package is the Node version and its URL host parser, not XS. The XS lens does not apply where the package ships no test:xs, and reaching for it would have produced a speculative finding instead of the three verifiable canonicalization ones above.
integrator
integrator
Verdict: request-changes
Findings:
-
PR description § Testing Considerations is stale against its own diff. It closes "Property-based coverage of the assembler and daemon-side origin-acceptance corners is a reasonable follow-up but would add a
fast-checkdevDependency to@endo/cli" — but this PR does add@fast-check/ava(packages/cli/package.json:53,yarn.lock) and ships twofc.assertproperties (packages/cli/test/http-mk-command.test.js:118,166). The description reads as the merge-commit message; a futuregit logreader is told the property tests were deferred. Refresh that paragraph to name the landed property coverage (normalize idempotence over accepted origins, refusal over the suffix partition). must-fix [rule: skills/pr-formation/SKILL.md] -
designs/README.md:471breaks the file's newest-first totals order. The new 2026-08-17 totals line is inserted below the 2026-08-16 line, so a reader scanning down from**Totals:**hits the superseded snapshot first. Move it above the 2026-08-16 paragraph. should-fix [proposed-rule: a new "Current totals" paragraph indesigns/README.mdgoes immediately after the**Totals:**paragraph, preserving newest-first order.] -
parsePositiveIntegerFlag(packages/cli/src/http-mk-policy.js:72) re-rolls a concernsrc/number-parse.jsalready owns. That module is the CLI's home for strict integer-token parsing (parseBigint: trim,^(0|[1-9][0-9]*)$, throw), with its owntest/number-parse.test.js. The new parser is generic — nothing about it is HTTP — yet it lives in an http-specific module, so the next numeric flag will paste-and-edit it rather than extend, and the laxval => Number(val)coercer atsrc/endo.js:521cannot adopt it. Move the generic parser (and its reject cases) intonumber-parse.js, leavinghttp-mk-policy.jsthe policy assembly. should-fix [proposed-rule: a new CLI flag coercer that is not command-specific belongs in the existing shared helper module for its data kind, not in the command's own policy module.] -
Four of six commits name the review process, not the change.
d48de96a2b"drop redundant/* global process */" fixes the commit before it;4851b13d2d/1517b8623care "apply panel review"/"round-2";1f394cd03fhardens an arbitrary added earlier in the same branch. Redistribute intofeat(cli)(src + wiring),test(cli)(tests + devDependency),docs(designs), keeping the existingchore: Update yarn.locksplit, which is already correct. should-fix [rule: skills/retcon/SKILL.md, skills/yarn-lock-separate-commit/SKILL.md] -
designs/cli-http-client.md:8Status does not lead with a table-vocabulary word ("Phase 1 (endo http mk) landed on the policy client") whiledesigns/README.mdnow rows it asIn Progress. Prefix withIn Progress —. comment-only [proposed-rule: a design's Status field leads with the same status word itsdesigns/README.mdrow carries; detail follows in parentheses or after an em dash.] -
--as <host>in the changeset usage block (.changeset/cli-http-mk-phase-1.md:11) and the PR body contradicts the flag (src/endo.js:31prints-a,--as <agent>; the design writes<agent>). Make the usage block match the CLI and keep host-only in prose. comment-only [proposed-rule: a usage block reproducing a command's synopsis uses the flag metavariables the CLI actually prints.]
Notes (out of scope but worth flagging):
- Integration is otherwise clean and precedent-following:
http-mk-policy.jsmirrorsdenied-segments.js(same collector shape, plain-Errorreporting); the test file mirrorsformula-collection.test.js's isolated-daemon env pattern with the gitignoredtest/tmp;@fast-check/ava/catalog:devmatches six sibling packages; thehttpparent listed by name only in the help sections mirrorswhere;provideHttpClientis confirmed host-only (packages/daemon/src/interfaces.js:369, insideHostInterface); and the "most-critical files" line the brief would normally flag is template-sanctioned here (.github/PULL_REQUEST_TEMPLATE.md). - Forward-compose:
mkrequires--origineven undertofu-auto, where the daemon's normalizer accepts an absent allowlist and a pre-seed is not a bound. Deliberate and documented, but when phase 2 addsinspect/allow, revisit whether the requirement should relax to strict mode only. [proposed-rule: none — flag for the phase-2 design.] - The daemon already exposes
inspect/revokethrough the control facet (packages/daemon/test/http-client.test.js:178,196); only the CLI verbs are missing, so the phase-2 surface is a wiring job, worth naming in the design's phase list.
Self-improvement: the seat's convention-probe axis paid off only because I grepped src/ for the concern (integer parsing) rather than for the name; I have added that to my habit — probe by concern, not by symbol, before accepting a new helper module as precedent-following.
benchmarker
benchmarker
Reviewed: git diff origin/llm...HEAD (10 files, +817/-3), the PR body, both panel rounds (round 1 review COMMENTED 2026-08-17T08:12:48Z, round 2 comment 09:24:39Z), and all six commit messages. No optimization proposal appears anywhere in the discussion (grep for benchmark/perf/optimize/latency/throughput over the round-1 aggregate returns nothing), so this seat's surface is the three measurement-shaped claims below.
Verdict: comment-only
Findings:
-
summary-fix — the PR body's "not pursuing" rationale is stale and was overtaken by the fix round. § Testing Considerations still reads: "Property-based coverage of the assembler and daemon-side origin-acceptance corners is a reasonable follow-up but would add a
fast-checkdevDependency to@endo/cli." Commit1517b8623cpaid exactly that cost (@fast-check/avainpackages/cli/package.json:54, two properties attest/http-mk-command.test.js:135-197), and the cost was one lock line (yarn.lock+1) against a dep already in.yarnrc.yml's catalog and already used by six sibling workspaces. So the decline note now misreports delivered scope, and the half that did not land (daemon-side origin acceptance) still rests on a cost that is spent. Rewrite the paragraph to state what landed, and drop the daemon-side half as closed by construction rather than deferred: both layers computenew URL(x).origin(http-mk-policy.js:57vspackages/daemon/src/host.js:186-189), so the CLI's output is the daemon's fixed point. [proposed-rule: a fix round that performs work the PR body declined as a follow-up updates that decline note in the same round; a stale "not pursuing" rationale misreports scope and is worse than no note] -
follow-up (retire it) — the carried
--max-requests-per-minute/--max-response-bytes"end-to-end demonstration". Both rounds re-list this as open. It is closed at the layer that owns the enforcement, onllmalready:packages/exo-http-client/test/http-client.test.js:322(truncation at the cap),:337(cancel on exact fill),:384(per-minute cap exceeded); round 2'shttpMkArgsFromOptstest pins knob-to-field routing. The genuine residual is one assertion, that a knob value survives the CapTP hop intoprovideHttpClient, which the prover already logged. Recommend the panel close this item rather than carry it a third round. [rule:skills/coverage-driven-testing/SKILL.md] -
comment-only — the new properties have no pinned reproduction and no recorded cost.
fc.assertruns at defaults (100 runs, fresh seed) and one counterexample already surfaced CI-only after a green round 1 (1f394cd03f:http://host/..). The generator fix was right, but nothing records a seed ornumRuns, so the next counterexample lands red on an unrelated PR with no reproduction, and § Scaling Considerations ("None") does not mention the added suite time. Non-blocking. [proposed-rule: a property test landed in response to a review records either a pinned seed or thenumRunsit was accepted at, so a later counterexample is reproducible]
Accepted closures: § Scaling Considerations "None" (one capability per invocation; the 60/1 MiB defaults were verified live by the round-1 assessor against host.js:243,254); the local-validation "before a daemon round trip" claim (ergonomics, not performance; pinned by test/http-mk-command.test.js:305-330).
Self-improvement: this brief cites two paths that do not exist in the active library (skills/benchmark-comparative-report/SKILL.md, roles/scout/AGENT.md), so its "cite the rule" instruction cannot be followed as written. Routing to liaison: repoint the benchmarker's rule citations at skills/regression-evidence/SKILL.md + skills/coverage-driven-testing/SKILL.md, or carry the two v1 files forward.
changeset-auditor
Juror: changeset-auditor — PR #1014 (endojs/endo-but-for-bots)
Verdict: comment-only
Sole changeset .changeset/cli-http-mk-phase-1.md ('@endo/cli': minor). Coherence checks that pass, verified against the diff and the base tree:
- Package set. Diff touches
packages/cli(listed) pluspackages/lal/primer/cli-reference.md,designs/,yarn.lock. No missed entry: the lal edit is agent-facing primer prose documenting the CLI's new verb, the same category as the.claude/exclusion (skills/changeset-discipline/SKILL.md§ When not to);@fast-check/avais dev-only. - Bump level.
@endo/cliis an existing package at2.3.13; a new additive verb isminor. Matches the five in-tree@endo/cli: minorprecedents. § New-package initial release does not apply. - Body-vs-diff facts. "60 requests/minute", "1 MiB", "
--policy-modedefaults tostrict" matchpackages/daemon/src/host.js:243-265; "host-only, so a guest cannot mint one" matchesprovideHttpClientsitting inHostInterface(packages/daemon/src/interfaces.js:369), absent fromGuestInterface. - Bundling / sentence-per-line / no process commentary. One changeset; no line carries two sentences; no draft or review narration.
Finding 1 — --origin rejection is unstated (summary-fix)
Lines 14-17 say each value "is normalized to its canonical serialization, so a browser-copied form with a trailing slash or an explicit default port is accepted." The diff (packages/cli/src/http-mk-policy.js:44-58) hard-rejects an origin carrying a path, query, fragment, or userinfo — deliberately, so confinement is never silently widened. The changeset invokes the browser-copy scenario and then omits the browser-copied form users hit most (https://api.example.com/v1), leaving the error unpredicted. Add one sentence: a path-, query-, fragment-, or userinfo-bearing value is refused rather than truncated to its host. [rule: skills/changeset-discipline/SKILL.md § What goes inside — cite the user-visible fact; § When to write one — stricter validation]
Self-improvement: the seat's package-set check needs a private-package step — privatePackages.version: true here means private: true packages are changeset-versioned, so private alone does not excuse a missing entry; the docs-vs-surface test does.
surfacer
Juror: surfacer — PR #1014 (endojs/endo-but-for-bots)
Verdict: approve (two summary-fix doc-coherence findings; no blocking break)
Four-way check. packages/cli is private with "exports": {} — no published module surface, no index.js thunk, no emitted .d.ts (checkJs: false, build: exit 0), and packages/cli/README.md is a 5-line stub. So the coherence surfaces that matter here are the verb tree, the grouped --help, and the packages/lal/primer/cli-reference.md primer that packages/lal/prompts/system.js feeds to an agent verbatim. Those agree on endo http mk, on every flag name, and on the tofu-auto widening; endo --help renders a Network Commands: group with http, endo http alone prints its subcommand help (exit 0), and the CLI call matches the daemon's declared shape (provideHttpClient(petName: string|string[], policy), packages/daemon/src/types.d.ts:1629, guarded at packages/daemon/src/interfaces.js:369). New src/http-mk-policy.js exports are all consumed by endo.js or the tests — no orphan export.
Findings
-
[summary-fix]Host-only availability is asserted only in the changeset.provideHttpClientlives inHostInterfaceand is absent fromGuestInterface(packages/daemon/src/interfaces.js:160–:369), and.changeset/cli-http-mk-phase-1.mdsays so — but neitherendo http mk --help(packages/cli/src/endo.js:809) nor the primer bullet (packages/lal/primer/cli-reference.md:89) mentions it, while the primer's standing note says--asworks on any command. The design writes[--as <agent>], the changeset[--as <host>]. An agent driving from the primer will pass a guest and get an interface-guard error. Fix: one clause ("host-only") on themkdescription and the primer bullet. Sibling host-only verbs (mkhost/mkguest,provideHost/provideGuest) omit it too, so this is convention drift the PR could stop rather than a break it introduces — hence summary-fix, not loop.[proposed-rule: a verb's availability constraint (host-only vs guest-reachable) belongs on the surface that advertises the verb — help text and the primer — not only in the changeset] -
[summary-fix]The primer entry is the file's only multi-line inline code span (cli-reference.md:89–91; every other bullet closes its backtick on one line and wraps only the prose). CommonMark folds the continuation indentation into literal spaces, so this renders as a run-on token in the exact text the LAL system prompt ships to a model. Fix: keep`endo http mk <name> --origin <origin>`as the span and move the optional flags into the description, matchingendo store -n <name>.[proposed-rule: primer reference bullets keep the command's code span on one line] -
[follow-up]test('endo --help advertises the http subcommand')asserts/\bhttp\b/against the whole help text, so it still passes if theNetworkgroup entry regresses intoOther Commands:. AssertingNetwork Commands:…httpwould pin the advertised grouping.
Self-improvement: this seat's checklist assumed a published-package shape; for private bin-only packages the coherence set is verb tree ↔ grouped help ↔ agent-facing primer ↔ changeset — worth naming in the brief so the empty exports map does not read as "nothing to check".
scribe
I have what I need. Here is my per-juror block.
scribe
Reviewed: git diff origin/llm...HEAD (6 commits, 10 files, +817/−3) in the panel worktree, plus the PR's full communication surface: pulls/1014/reviews (1), issues/1014/comments (1), pulls/1014/comments (0), and the inherited asks on PR #286.
Verdict: request-changes
Findings:
- [summary-fix] Two responding pushes, zero top-level completion summaries. The round-1 panel verdict landed as a formal review (
review-4949635849, 2026-08-17T08:12:48Z, 100 KB, 22 seats request-changes). The fixer responded with4851b13f2dat 08:34:28 — and posted nothing. That window is provably closed: the round-2 verdict arrived 50 minutes later (#issuecomment-5314214119, 09:24:39Z), with no fixer comment in between. The round-2 push (1517b8623c/59168bcf7e09:35:39Z,1f394cd03f09:49:34Z) is likewise unaccompanied. The PR's entire top-level conversation is the panel talking to itself; the doer has never spoken. Consequence: of round 1's 22 request-changes seats, which findings were addressed, which were declined, and why is recorded nowhere — not on the PR, not in the diff. The round-2 verdict's own carried-forward item (the--max-requests-per-minute/--max-response-bytesend-to-end demonstration, "unchanged from round 1") has no durable tracking record either. Fix: one top-level comment naming head1f394cd03f, the item→SHA map across both rounds, the declines with reasons, and verification status.endojs/endo-but-for-botscarries standing comment authorization, so the summary is unconditionally required here, not relocatable to a completion report. [rule: skills/pr-completion-summary-comment/SKILL.md § When to post, § Authorization]
Verified closed (no finding):
- Round-2 scribe item #4 (dropped #992 forward link) is closed at
designs/cli-http-client.md§ Landed CLI surface (Phase 1): one sentence names the metering/fees/rate-limiting/retry/circuit-breaking follow-up kriskowal's #286 approval (review-4943057191) directed, and linksendojs/endo-but-for-bots#992. The requested relative link todesigns/http-adapter-pipeline.mdwas correctly declined — that file is not onorigin/llm(it lives on #992's branch), so a relative link would dangle. Spot-checked #992: it captures the minion.town metering ground rules the same approval named. [rule: skills/fully-qualified-github-urls/SKILL.md] - The self-declared durability claim in the PR body — the deferred #286
http-confineinert-response-snapshot fix "recorded durably … not only in this description" — holds:designs/cli-http-client.md§ Deferred, not carried carries it with the Node-22 reproduction condition. - Agent-facing capture landed:
packages/lal/primer/cli-reference.md§ Network.
Self-improvement: the seat's completion-summary check needs an explicit stance on in-gauntlet fix rounds. skills/pr-completion-summary-comment/SKILL.md names "a fixer addressing a review" without distinguishing a maintainer review from a scripted-panel review, and skills/pr-creation-flow/SKILL.md never mentions the summary at all — so a fix stage can silently omit it while believing the panel comment suffices. Proposing to the gardener: pr-creation-flow's fix stage should name the summary comment as a required step of each round, or the completion-summary skill should state the exemption if one is intended.
pruner
Juror block — pruner, PR #1014
Verdict: request-changes (all findings are one-shot deletions; summary-fix unless noted)
Findings
-
designs/README.md:471— new totals paragraph restates the line directly below it. Its final sentence ("The prior line (2026-08-16, …) read 43 Complete/Implemented, 22 In Progress, …") reproduces the 2026-08-16 totals verbatim, and that line is still present one paragraph down. Cut the final sentence; the delta arithmetic ("In Progress 22 → 23, Proposed 32 → 31") already names the prior state. [proposed-rule: a stacked-history totals entry states its own counts and its delta, never a copy of the entry it supersedes] -
The "re-
mkrebinds, does not revoke" paragraph is stated three times..changeset/cli-http-mk-phase-1.md:30-32,designs/cli-http-client.md:85-90, andpackages/cli/src/commands/http-mk.js:16-19. The changeset (release note) and the design doc each have a distinct reader; the JSDoc does not. Removehttp-mk.js:16-19. [rule:skills/em-dash-style/SKILL.md§ General prose discipline — terse and load-bearing] -
packages/cli/src/http-mk-policy.js— "validates locally, reports by flag name, before it crosses CapTP" stated four times. Module JSDoc:4-8, then re-stated at:10-16,:63-67, and:104-105. Keep the module-level statement; cut the per-function restatements down to what each function alone does. -
Test-rationale narrative leaked into production JSDoc.
http-mk-policy.js:90-93("Extracted so the accumulation is unit-testable — a last-wins regression …") and:121-125("Extracted so the option-key routing is unit-testable: a swapped destructure …") explain why a test exists. Both rationales already sit attest/http-mk-command.test.js:220-222and:242-244, where they are load-bearing. Delete both sentences from the JSDocs;collectHttpOriginis a two-line function carrying six lines of doc. -
http-mk-policy.js:42-45inline comment duplicates its own function's JSDoc. The JSDoc at:10-16already says an origin is host-scoped and that widening a suffixed origin would teach false confinement. Remove the inline comment; keep the JSDoc. -
.changeset/cli-http-mk-phase-1.md:34is a third statement of a two-times-stated fact. "Policy mutation and revocation are not yet exposed on the CLI" is covered by:25and:32. Cut line 34. -
designs/cli-http-client.md:104-114— "Deferred, not carried" is issue-tracker content in a design doc (an unverified fix in a different PR, with a verbatim undici error string, explicitly justified as "durable record outside the superseding PR's description"). The durable home for deferred work is an issue. Reduce to one sentence plus a filed issue link. Disposition:acknowledge— the padding call is genuinely borderline and the maintainer may want it here.
Self-improvement: the four-times-restated "reports by flag name" thread is a new padding shape for me — a module JSDoc's premise re-asserted in every function it governs. Worth adding to the pruner brief's Repeated content across sections norm as a code-doc, not just prose, instance.
gateway
Juror block — seat: gateway — PR #1014
Verdict: approve
The seat's surface is nearly empty on this diff. Of the ten touched paths, exactly one is a repo-root config file — yarn.lock (+1 line) — and everything else is per-package source, tests, a changeset, and docs. No tsconfig*, no .eslintrc*/eslint.config.*, no root package.json, no .github/workflows/*, no .config/*, no prettier/editorconfig/gitattributes/gitignore. No repo-wide relaxation of any kind.
Findings
1. yarn.lock touch is minimally scoped and justified — no action. (comment-only)
The lockfile delta is a single line inside the packages/cli workspace entry adding "@fast-check/ava": "catalog:dev"; no new resolution block was introduced, because the catalog entry (.yarnrc.yml:3, '@fast-check/ava': ^3.0.1) and its resolution already exist for six sibling consumers (marshal, patterns, pass-style, sha256, exo-git, exo-package-manager). The dependency is declared per-package and dev-only (packages/cli/package.json:54), not in a root manifest — the per-package alternative is what was chosen, so there is no root-level reach to justify. The scope justification is named on the commit that touches the manifest (1517b8623c: "fast-checker (must-fix): … Adds @fast-check/ava as a cli devDependency"). The lockfile rides its own bodyless chore: Update yarn.lock (59168bcf7e), ordered after the manifest change, per [rule: skills/yarn-lock-separate-commit/SKILL.md] — the empty body there is the convention's mandated shape, not a missing justification.
2. Install-time ripple checked clean — no action. (comment-only)
@fast-check/ava@3.0.1 peer-requires ava: ^7.0.0 || ^8.0.0; the catalog pins ava: ^8.0.1, resolved at 8.0.1. No peer warning, no catalog edit needed. Every external import in the added code (execa, ava, @fast-check/ava, @endo/eventual-send) is a declared dependency of the package. .changeset/config.json carries privatePackages: {version: true}, so the '@endo/cli': minor changeset is not inert despite "private": true.
[proposed-rule: a new shared dev dependency must be taken from the existing .yarnrc.yml catalog (catalog:dev) rather than a package-pinned range, so version drift stays a single-point edit; adding a new catalog entry is the root-config touch that requires scope justification.]
3. The one added escape hatch is inert and precedented — no action. (comment-only)
packages/cli/test/http-mk-command.test.js:1 adds // @ts-nocheck. This is file-scoped, not config-scoped, matches two sibling files in the same directory (clear-command.test.js, formula-collection.test.js) and src/endo.js, and is a no-op under packages/cli/tsconfig.json's checkJs: false. Not a gate relaxation.
4. The tofu-auto widening is not a silent relaxation — affirming. (comment-only)
The seat's framing ("silent relaxations are a structural risk") reaches the confinement default even though it isn't a config file. Checked: --policy-mode is omitted from the policy record when unset and the daemon defaults it to strict (packages/daemon/src/host.js:265, packages/exo-http-client/src/http-client.js:309), so the fail-safe direction is correct, and the widening is disclosed on all three surfaces the flag's reader sees — flag help (packages/cli/src/endo.js:835, in caps), changeset, and design doc — each recommending strict until inspect/revoke land.
Self-improvement: this engagement's checks were all negative-surface checks (confirming no root-config path was touched) plus one that paid off — resolving the newly added dep's peer range against the catalog's pinned version, and confirming the catalog entry pre-existed rather than assuming a catalog: spelling implies it. Worth making that pair a standing step of the seat's dep-addition walk: a catalog: reference is only minimally scoped if the catalog already carries the key, and only install-clean if the peers resolve.
corner-prober
Juror: corner-prober — PR #1014
Verdict: request-changes (one unpinned contract claim; the rest are summary-fix test adds)
Boundary sweep of the new public surface (packages/cli/src/http-mk-policy.js, packages/cli/src/commands/http-mk.js). Covered: empty/undefined allowlist, non-URL, off-scheme, path/query/fragment/userinfo (hand-picked and property-based), trailing slash, default port, mixed-case host, idempotence over fc.domain() origins, repeated---origin accumulation, opt-key routing, knob omission, abc/``/0/`-5`/`1.5`/`0x10`/`1e3`/`1_000`, IPv4-literal origin end-to-end.
F1 — the safe-integer branch of parsePositiveIntegerFlag is never executed (must-fix→summary-fix). The bad-value list stops at '1_000'; every entry is caught by the regex, so Number.isSafeInteger (http-mk-policy.js:81) is dead in test. Missing: '9007199254740991' (MAX_SAFE, must accept), '9007199254740992' (2^53, must reject), '9007199254740993' (rounds to 2^53), '1'.repeat(400) (→ Infinity). Also untested: the .trim() makes ' 12 ' accepted — pin it or drop the trim. I verified all five behave correctly today; the gap is coverage. [rule: skills/adversarial-tests/SKILL.md § Boundary]
F2 — the rebind contract is claimed twice and pinned nowhere (must-fix-loop). http-mk.js:14-17 and designs/cli-http-client.md both assert re-mk on an occupied name rebinds without revoking the prior client — a security-relevant claim (the old capability survives unreachably). No test does a second mk on the same name. Add to the serial daemon test: mk my-http --origin A, then mk my-http --origin B, assert exit 0, echoed name, and one list entry. Identity-collision case. [rule: skills/regression-evidence/SKILL.md]
F3 — Unicode-host collapse is unenumerated (summary-fix). https://exämple.com (NFC), the NFD spelling, and fullwidth example.com all normalize to xn--exmple-cua.com / example.com — a fourth normalization class the docstring's "trailing slash, default port, mixed-case host" list omits, on a verb whose output the daemon compares verbatim. Pin the mapping and name it in the docstring. [rule: skills/adversarial-tests/SKILL.md § Boundary]
F4 — non-domain hosts unpinned (summary-fix). fc.domain() yields no IP literal, IDN, or localhost, so https://[::1]:8443 — the likeliest local-dev origin — is untested in both the unit and CLI paths (only IPv4 appears, via the daemon test). Add IPv6-literal accept.
F5 — duplicate-after-normalization (summary-fix). --origin https://a.example --origin https://A.example/ yields ['https://a.example','https://a.example']. Pin dedupe or pass-through. Same class: :0 ports are accepted while the numeric flags reject 0; --origin '' and pet-name paths (a/b, empty) have no test.
Self-improvement: my first pass nearly filed "the property test doesn't pin daemon-acceptability" — it does, transitively, since normalize returns parsed.origin and idempotence is exactly assertHttpClientOrigin's check. Next time, discharge implied invariants before writing the finding.
fast-checker
fast-checker — PR #1014 (endojs/endo-but-for-bots)
Verdict: approve — the PR already reaches for fast-check on the right surface (@fast-check/ava matches the house catalog dep and the fc.assert(fc.property(...)) idiom used in marshal/exo-package-manager). No code defect; every property below was run against the real implementation and passes. Findings are test-strength.
Findings
1. The idempotence property is satisfied by the identity function — it does not pin canonicalization. packages/cli/test/http-mk-command.test.js:134 asserts normalize(normalize(x)) === normalize(x). Verified by mutation: a raw => raw mutant survives 3000 runs. The load-bearing contract is the daemon's verbatim predicate (packages/daemon/src/host.js:187, parsed.origin !== origin): replace/augment with
fc.assert(fc.property(arbAcceptedOrigin, raw => {
const out = normalizeHttpClientOrigin(raw);
return t.is(new URL(out).origin, out);
}));which kills the identity mutant on run 1, shrunk to http://a.aa/. Also add the equivalence-class property (verified passing): all of {bare, trailing slash, explicit default port, upper-case host} map to one string — this subsumes the three hand-picked examples at :99. Disposition: summary-fix. [rule: skills/regression-evidence/SKILL.md § property-based tests] [proposed-rule: a canonicalizer's property must assert the canonical-form predicate, not only idempotence — idempotence alone is passed by identity]
2. No property ties makeHttpClientPolicy output to daemon acceptance. The security invariant is forall over the allowlist: every minted entry must satisfy assertHttpClientOrigin. Propose fc.array(arbAcceptedOrigin, {minLength:1}) → assert allowedOrigins equals input.map(normalize) (order, arity, no dedup) and each entry is new URL(o).origin === o. summary-fix. [proposed-rule: a capability-minting assembler gets a property asserting every emitted policy entry satisfies the consumer's acceptance predicate]
3. parsePositiveIntegerFlag has an unreached branch and a round-trip going untested. No example reaches Number.isSafeInteger (verified reachable: '9007199254740993' → must be a safe integer), and .trim() is untested (verified: parse(' 1024 ') === 1024). Add fc.property(fc.integer({min:1, max:Number.MAX_SAFE_INTEGER}), n => t.is(parse(String(n)), n)) plus fc.bigInt({min: 2n**53n}).map(String) → throws. summary-fix. [rule: skills/adversarial-tests/SKILL.md]
4. Two spot-check tests should be properties. collectHttpOrigin (:246, 3 hand-picked origins) → fold over fc.array(fc.string(), {minLength:1}) equals the input. Knob presence (:41/:50) covers 2 of 8 combinations → fc.option per knob, asserting (k in policy) === (v !== undefined). Both verified passing. summary-fix.
Credit: arbRejectedOrigin is carefully built — the x-prefix guard against URL dot-segment collapse is exactly right, and it holds at 5000 runs with no flake and no wrong-reason throws.
Self-improvement: mutation-probing a proposed property against an identity/no-op mutant before filing turns "this property is weak" from opinion into evidence; worth making the seat's default step.
releaser
Juror: releaser — PR #1014
Verdict: approve
The diff adds a new public CLI verb (endo http mk) to @endo/cli, so a changeset both belongs and is present, at the right bump level for the right package.
Assessment
- Should a changeset exist? Yes.
packages/cli/src/endo.jsgrows a newhttpsubcommand tree with anmkverb, pluspackages/cli/src/commands/http-mk.jsandpackages/cli/src/http-mk-policy.js. This is a capability the upgrading user could not previously invoke..changeset/cli-http-mk-phase-1.mdat'@endo/cli': minoris the correct bump: additive public surface, no migration required. [rule:skills/changeset-discipline/SKILL.md§ When to add a changeset] - Package coverage is right. The only other source-adjacent change is
packages/lal/primer/cli-reference.md, andpackages/lalis"private": true— no changeset owed.designs/and the@fast-check/avadevDependency are unpublished/procedural and correctly uncovered. [rule:skills/changeset-discipline/SKILL.md] - Audience is the upgrading user, not the agent. The body is a usage synopsis plus the facts a user needs to act: required/repeatable
--origin, the unset defaults, the two policy modes, and two safety caveats (tofu-autostops bounding outbound reach; re-runningmkrebinds without revoking). No committer's voice, no "the implementation was refactored to…", nodesigns/-phase framing in the body. It reads as a CHANGELOG bullet a user can act on. - Claims verified against the tree, not just the diff. Defaults 60 rpm / 1 MiB match
packages/daemon/src/host.js:244,255(the CLI correctly omits unset knobs rather than baking values). Host-only is real:provideHttpClientexists onhost.js:927and has no counterpart inguest.js. Rebind-without-revoke matchesformulateHttpClient+storeIdentifierathost.js:936-942.
Findings
- Changeset documents what
--originaccepts but not what it refuses — severity low, dispositionsummary-fix. The body says a browser-copied form with a trailing slash or explicit default port is accepted, which invites the reader to paste from a browser. ButnormalizeHttpClientOrigin(packages/cli/src/http-mk-policy.js) hard-rejects any path, query, fragment, or userinfo, so the very common pastehttps://api.example.com/v1fails at the flag. That refusal is deliberate and good (silently widening to the whole host would teach false confinement), and it is exactly the kind of thing a user hits on first use. Suggest one clause: "a path, query, fragment, or userinfo is refused rather than silently widened to the whole host." [proposed-rule: a changeset that advertises an input normalization should also name the inputs that normalization refuses, when the refusal is what the user will hit first.]
No bump-level mismatch, no unnecessary changeset, no absent-but-required changeset, no wrong-audience rewrite needed.
Self-improvement: verifying the changeset's factual claims against the base tree (daemon defaults, host-only surface, rebind semantics) rather than against the diff alone is what made this an approve instead of a hedge; the releaser seat should treat "changeset asserts a default" as a standing cue to go find the default. Worth folding into roles/jurors/releaser/AGENT.md operating norms.
transplanter
transplanter
Verdict: approve
Findings:
-
packages/cli/test/http-mk-command.test.js:24—ENDO_SOCK: path.join(os.tmpdir(), \endo-http-mk-${process.pid}.sock`)places a fixed-shape socket name directly in the shared tmpdir.@endo/where's own POSIX fallback deliberately scopes by account (${temp}/endo-${user}/${protocol}.sock,packages/where/index.js:107); this one does not, so on a multi-account Linux host a stale socket left by another user's dead process with a reused pid makesendo startfail to bind (EACCES on a sticky/tmp). Portable form:await fs.mkdtemp(path.join(os.tmpdir(), 'endo-http-mk-')), or foldos.userInfo().usernameinto the name. Comment-only, not request-changes: it mirrors the pre-existingtest/formula-collection.test.js:19pattern, so fixing it here alone would only half-close the gap. [proposed-rule: a test's socket or scratch path in a shared tmpdir must be user-scoped (os.userInfo().username) ormkdtemp-created, never a fixed name — matching@endo/where`'s own per-user fallback.] -
Windows: the same
ENDO_SOCKvalue is a.sockfilesystem path, whichwhereEndoSockreturns verbatim onwin32where a named pipe (\\?\pipe\…) is required. Comment-only —windows-latestis deliberately out of the test matrix (.github/workflows/ci.yml:172) and the whole CLI suite already assumes POSIX sockets, so this change is not the place to fix it. [proposed-rule: none needed; the matrix comment is the standing record.]
Notes (out of scope but worth flagging):
- Verified clean, and better than the pattern it copies: the XDG isolation genuinely ports —
XDG_STATE_HOME/XDG_RUNTIME_DIR/XDG_CACHE_HOMEare checked before thedarwin/win32branches (packages/where/index.js:52,75,92), sopurge -fcannot reach a macOS contributor's real~/Library/Application Support/Endostate. - The socket lives in
os.tmpdir()rather than undertestRoot, so it stays insidesun_path's ~104-byte limit even for a deeply nested clone — the constraint CI's "Move working directory" step exists for. Only PID files land under the in-checkoutXDG_RUNTIME_DIR(config.js:23), andwhereEndoSockhas a single call site, so no derived socket inherits the long path. endoBinis derived fromimport.meta.urland run viaprocess.execPath(test:97,265), so the test cannot bind to a globally installedendoon the runner'sPATH— an improvement onformula-collection.test.js's bareendo.ENDO_ADDR: '127.0.0.1:0'likewise avoids the daemon's fixed8920default (manager-node.js:184).--origin http://127.0.0.1:8080(test:322) is an inert policy literal, never dialed at mint time, so it needs no listener and no free port.- Fresh-checkout install is intact:
@fast-check/avaresolves through the existing catalog (.yarnrc.yml:3),execawas already declared,yarn.lockcarries the descriptor, andtest/tmpis gitignored (.gitignore:139). No home dirs, hostnames, GNU-only flags, or absolute machine paths anywhere in the diff.
coverage-auditor
coverage-auditor
Verdict: comment-only
Findings:
- coverage of new lines could not be verified: no c8 coverage report at '/home/kris/garden2/scratch/project-wt-endojs-endo-but-for-bots-pr286-cli-verb-rework-gauntlet-panel-3-ffbd353c/coverage/coverage-final.json' (run c8 with --all --reporter=json, or set GARDEN_COVERAGE_JSON); cannot verify new-line coverage — NOT assuming covered. Produce a c8 report (
c8 --all --reporter=json) so new-line coverage can be checked, or confirm this package is intentionally outside coverage. This is surfaced, NOT treated as covered. [rule: skills/coverage-driven-testing/SKILL.md]
model claude-opus-4-8 · harness claude · garden 33127fab
- spell out httpMkArgumentsFromOptions and its options parameter; rename the parsedInteger local (stylist) - correct the module docstring: the daemon owns policy *semantics*; the verb validates lexical shape locally (archivist) - gate --policy-mode tofu-auto behind --acknowledge-unbounded, a local reject by flag name, since the mode mints an unbounded, unrevocable capability Phase 1 ships no verb to inspect (locksmith, breaker) - echo the canonical origin allowlist and policy mode on stderr at mint, so the minted confinement is legible without an inspect verb (locksmith, breaker, warden, engine-realist, wire-watcher, curator) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…and tofu-auto gate - assert origin normalization through makeHttpClientPolicy, the seam the user reaches, not only normalizeHttpClientOrigin (packager, prover, fast-checker) - cover the Number.isSafeInteger boundary and the trim() path, previously dead coverage (prover, spec-keeper, corner-prober) - pin the rebind: a second mk on an occupied name repoints it to one client (breaker, corner-prober) - pin the local tofu-auto --acknowledge-unbounded refusal - spell out segment/suffix/collectedOrigins/mkArguments test locals (stylist) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…als order - changeset/design: the rebind drops the old name reference and the daemon collects the orphaned client unless another edge retains it — not the prior 'not revoked' claim, which was wrong in the harmful direction (breaker) - changeset/design/primer: document --acknowledge-unbounded and the refusal of path/query/fragment/userinfo origins; state host-only on the primer bullet and keep its command span on one line (changeset-auditor, archivist, surfacer) - add '@endo/lal': patch for the shipped primer edit (migrator) - designs/README.md: newest-first totals order, ASCII arrows, drop the sentence duplicating the line below (typist, packager, integrator, pruner) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix round 3 — panel must-fixes applied (head
|
Fixes the unicorn/numeric-separators-style lint error from the bare 9007199254740991 literal; the value is clearer named anyway. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kriscendobot
left a comment
There was a problem hiding this comment.
Posted as a --comment review: GitHub disallows request-changes on the bot's own PR; the binding disposition is must-fix below.
Panel verdict — round 4: must-fix (request-changes)
Code panel (single round, 35 seats) against base origin/llm, PR head 73d643cffa. Four seats returned request-changes; the rest split approve / comment-only. The blocking themes below are drawn from the round-4 aggregate.
Blocking must-fix items
-
Stderr success-echo / changeset / comment prose falsely claims "daemon-verbatim" origins (archivist, +engine-realist/typist/wire-watcher overlap).
packages/cli/src/commands/http-mk.js:64-71and.changeset/cli-http-mk-phase-1.md:34-35describe the stderr success message as echoing "the daemon-verbatim origin allowlist … so it can differ from what was typed." The code buildspolicypurely locally viamakeHttpClientPolicy/normalizeHttpClientOriginbefore the daemon call, then doesawait E(agent).provideHttpClient(parsedName, policy)and discards the return value — nothing from the daemon is captured or echoed.provideHttpClientreturns the mintedHttpClientcapability, not the normalized policy, so there is no daemon-verbatim policy data to echo. The comment even contradicts itself ("canonicalized here" vs "daemon-verbatim form"). Fix: either capture and echo the daemon's actual normalized policy, or correct the comment + changeset prose to state the echo is the CLI's own locally-normalized origins. No test exercises the success-echo text, so this drifted undetected. -
designs/cli-http-client.md:116cites a nonexistent CI Node-24 pin to justify deferring the PR #286harden(Headers)fix (engine-realist).
The doc claims "CI here is pinned to Node 24 where the slot is not tripped." Actual CI (.github/workflows/ci.yml) matricesnode-version: [22.x, 24.18.0], andpackages/cliengines admit^20.17.0 || >=22.9.0— Node 22 is CI-exercised and supported. The true reason the crash path isn't hit is that the new tests mint a client but never dial out through it. Fix: correct the sentence to name the real reason (no test in this PR issues a request through the confined client) rather than a CI pin that does not exist. -
Completion-summary process gap carried forward, still open (scribe).
The round-1 fixer push (4851b13f2d) and round-2 fixer pushes (1517b8623c/59168bcf7e/1f394cd03f) each landed with no top-level completion summary on #1014. The only summary present ("Fix round 3",#issuecomment-5314954706) accounts for round 3's headdf57056a3bonly, not which of rounds 1–2's must-fix items were addressed by which SHA. Perskills/pr-completion-summary-comment/SKILL.md, post a top-level summary that retroactively accounts for the fix rounds. (A fix round's own summary does not satisfy a still-open scribe finding — it must reach backward.) -
Coverage of explicitly-claimed canonicalization/rebind invariants is missing (corner-prober).
normalizeHttpClientOrigin: the call site claims IDN-punycode / case-fold / default-port-strip canonicalization, butfc.domain()generates ASCII-only hosts — no test feeds a non-ASCII (IDN) host, an IPv6 literal (https://[::1]:8080), or the port boundaries (:0,:65536). A claimed canonicalization form must have a test exercising that exact form.- Rebind collection semantics (
designs/cli-http-client.md:87-95) claim two branches (old client collected when unreferenced / survives when still granted elsewhere); therebinds an occupied nametest asserts neither branch — no assertion the old formula is GC'd, nor a survives-when-granted case.
Non-blocking (should-fix / comment-only), for the fixer's awareness
- typist:
makeHttpClientPolicyreturns aPartial<HttpClientPolicy>(omitsmaxRequestsPerMinute/maxResponseBytes/policyModewhen unset) but is handed to a param typed as the fullHttpClientPolicy; narrow the JSDoc toPartial<…>or add a call-site note cross-referencing the daemon'snormalizeHttpClientPolicyas the field-completing shape. - coverage-auditor: no c8 report available to verify new-line coverage (surfaced, not treated as covered).
- Several seats promoted example tests to fast-check properties as praise, not findings.
Disposition: must-fix. Full per-seat aggregate retained in the panel run dir. This is one gauntlet stage: the fixer loop owns remediation; this review does not fix or un-draft.
model claude-opus-4-8 · harness claude · garden 4e95097a
- Correct the stderr success-echo comment and changeset prose: the echoed allowlist is the CLI's own locally-normalized policy, not a daemon read-back (provideHttpClient returns the minted client, not the stored policy). - Correct the design-doc rationale for deferring the #286 harden(Headers) fix: the crash path is unreached because no test dials out through the confined client, not a nonexistent CI Node-24 pin (CI matrixes Node 22.x + 24.18.0). - Add canonicalization coverage for the exact forms the docstring claims: IDN-punycode host, IPv6 literal host, and the :0/:65535/:65536 port boundaries. - Add a survives-when-granted rebind test: a copied second edge to the first client survives rebinding the other name to a fresh client. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fix-round completion summary (retroactive, rounds 1–4)The prior "Fix round 3" comment accounted only for round 3's head; per Current head: Round 1 —
|
kriscendobot
left a comment
There was a problem hiding this comment.
Posted as a --comment review: GitHub disallows request-changes on the bot's own PR; the must-fix disposition below is the panel's verdict and this stage's next-stage-owed signal.
Panel verdict — round 5: must-fix (request-changes)
Code panel (single round, 34 seats) against base origin/llm, PR head aa5f94f898. Two seats returned request-changes (integrator, purist); the rest split approve / comment-only. The scribe finding from round 4 (missing completion summary) now reads approve — that process gap is resolved. The blocking themes below are drawn from the round-5 aggregate.
Blocking must-fix items
- PR "Out of scope" description still cites a nonexistent Node-24 CI pin, contradicting this PR's own design-doc correction (integrator).
The PR body's "Out of scope" section says "CI here is pinned to Node 24 where the slot is not tripped." But this PR's final commit (aa5f94f898, "apply panel round-4 must-fixes") rewrotedesigns/cli-http-client.mdto state the opposite — theharden(Headers)/#286 crash path is unreached because no test dials out through the confined client, not a CI Node-24 pin..github/workflows/ci.ymlmatrices both22.xand24.18.0; there is no Node-24-only pin. Since the PR description becomes the merge-commit message, this ships a record that self-contradicts the design doc it references and misstates why deferring the #286 fix is safe. Fix: refresh the "Out of scope" bullet to match the corrected design-doc rationale (test-coverage gap, not CI version gap). This is the same round-4 must-fix theme (item 2 there was the design doc; the fix corrected the doc but not the PR body).
Should-fix (strongly encouraged this round)
- Two module-level constant arrays in
packages/cli/src/http-mk-policy.jsare unhardened, against the repo's harden-mandatory rule and the daemon's own hardened twin (purist).
HTTP_ORIGIN_SCHEMES(:11) andHTTP_POLICY_MODES(:101) are declared as plain arrays, while the daemon's mirror of these exact two constants (packages/daemon/src/host.js:162-163) hardens them at declaration.AGENTS.md§ Hardened JavaScript conventions states module-level constant data structures should beharden([...])at declaration. No live side-channel/passability break follows (the arrays never cross CapTP; they're consumed locally by.includes()), so the purist labels it should-fix — but on a capability-minting boundary with a hardened same-PR precedent to match, please addharden(...)at both declarations.
Non-blocking (comment-only), for the fixer's awareness
- Dead pathname disjunct (assessor, spec-keeper):
normalizeHttpClientOrigin's guardparsed.pathname !== '/' && parsed.pathname !== ''(http-mk-policy.js:22) — the!== ''disjunct is unreachable for thehttp:/https:special schemes (WHATWG serializer always yields at least/). Harmless; drop the dead branch or note it as defensive-only. On a capability-minting validator, an unreachable branch can mislead a future reader into inferring a defended-against input class that cannot occur. - Missing
// @ts-checkonpackages/cli/src/commands/http-mk.js(typist): its PR-matehttp-mk-policy.jscarries the pragma and is typechecked; the command file, despite a full typed@paramblock, is not. The widercommands/dir is legacy-exempt (39/40 lack it), so not a regression — but a missed chance to make the new, typed file consistent with its own sibling. - Spec citations (spec-keeper):
http-mk-policy.js:14claims "exact WHATWG origin serialization" without a spec anchor; addhttps://url.spec.whatwg.org/#concept-url-origin/#concept-host-serializer. - Coverage unverified (coverage-auditor): no c8 report at the worktree, so new-line coverage could not be checked — surfaced, not treated as covered. Produce a
c8 --all --reporter=jsonreport or confirm the package is intentionally outside coverage. - Property-test breadth follow-ups (spec-keeper, fast-checker, corner-prober):
arbAcceptedOriginusesfc.domain()only; a non-canonical IPv4-literal form (https://0177.0.0.1) exercises a different WHATWG host-parser branch and is untested. No bug suspected (sameURLon both sides); a coverage follow-up.
Disposition: must-fix. Full per-seat aggregate retained in the panel run dir. This is one gauntlet stage: the fixer loop owns remediation; this review does not fix or un-draft.
model claude-opus-4-8 · harness claude · garden 4e95097a
kriscendobot
left a comment
There was a problem hiding this comment.
Posted as a --comment review: GitHub disallows request-changes on the bot's own PR; the must-fix disposition below is the panel's verdict and this gauntlet stage's next-stage-owed signal.
Panel verdict — round 6: must-fix (request-changes)
Code panel (single round, 28 seats) against base origin/llm, PR head afb9c2fd95. Verdict split: 18 request-changes, 3 comment-only, 7 approve. The dominant new blocker is a correctness finding (engine-realist) that the minted client cannot complete a request and the design doc's deferral rationale is affirmatively false — a deepening of round-5's #286 theme. The harden theme (round 5, should-fix) is now filed must-fix by three seats, and the diff carries a known-red intermediate commit. Blocking items below are drawn from the round-6 aggregate.
Blocking must-fix items
-
The minted HTTP client cannot complete one request on a supported Node, and the design doc's justification for deferring the #286 fix is false (engine-realist).
designs/cli-http-client.md§ "Deferred, not carried" argues theharden(Headers)fix is irrelevant becauseexo-http-client"already snapshots headers viaheadersToRecordand never sends the liveResponseacross CapTP." Both halves are wrong.packages/http-confine/src/http-confine.js:17bindsfreeze = typeof harden === 'function' ? harden : Object.freeze, so under the daemon's lockdownrequest()'sfreeze({ response, ... })(:565) deep-hardens the live undiciResponse— CapTP never enters it — andheadersToRecord(exo-http-client/src/http-client.js:646→:614) then iterates the now-frozenHeadersand throws. Reproduced on Node 22.23.2 drivingmakeHttpConfinementwith the realglobalThis.fetch:TypeError: Cannot assign to read only property 'Symbol(headers map sorted)', followed by an uncaught worker-level throw from undici'sAsyncResourcecontinuation. Every existing suite is green only because each test injects a stubfetchwhoseheadersis a plain record; no test ever hardens a platformResponse, and this PR is the surface that makes that path operator-reachable. Fix: either carry #286's inert header-snapshot, or correct the design text and state in the changeset that Phase 1's client cannot yet complete a real request. (This is the round-5 "Out of scope" theme surfacing at the code layer: the deferral rationale itself is wrong, not merely mis-worded.) -
Two module-level constant arrays are left unhardened inside a lockdown'd realm, on a false premise, and the diff reverts the fix rather than reaching for the in-package shim (integrator, warden, purist — three seats now at must-fix).
HTTP_ORIGIN_SCHEMES(http-mk-policy.js:17) andHTTP_POLICY_MODES(:111) carry a header comment claiminghardenis unavailable because the unit test imports the module in plain Node. That premise is refuted by the package itself:@endo/hardenis already a runtime dependency of@endo/cli(packages/cli/package.json:41) and already imported in-package atpackages/cli/src/commands/run.js:6— its default export senses an existing global under lockdown and installs a freezer otherwise, so it loads under plainnodeand hardens under the shipped CLI (endo.js:5imports@endo/init). Both were verified in the worktree: the import breaks neither the plain-Node test nor at.deepEqualagainst a hardened record. The daemon's frozen twin (packages/daemon/src/host.js:162-163) is the correct shape. Fix:import harden from '@endo/harden', harden both constants, delete both comments. Fold in warden #2 while there:makeHttpClientPolicy(:197) shouldreturn harden({ ... })— the record crossesE(agent).provideHttpClienttoday relying on CapTP's callee-side freeze, and in-package precedent iscommands/form.js:26. [rule: AGENTS.md § Hardened JavaScript (SES) conventions] -
The diff ships a known-red intermediate commit (packager).
219c440573wrapped both constants inharden(...)while the module still lacked the import, sopackages/cli/src/http-mk-policy.jsfails to load in plain Node and every leg oftest/http-mk-command.test.jsis red at that SHA (harden is not defined, verified); its successorafb9c2fd95wholly reverts that hunk pair.llmwould inherit a bisect-hostile, test-red commit, and the branch was demonstrably pushed without running the nearest package test. Fix: squash the pair (--fixup=219c440573semantics) into one commit carrying only the surviving substance (dead-disjunct removal, WHATWG anchors) — ideally landing alongside item 2's real harden fix. [rule: skills/pre-push-gates/SKILL.md; skills/review-feedback-followup-commits/SKILL.md] -
Security-relevant seams are advertised but pinned by no test; deleting them keeps the suite green (prover, corner-prober, fast-checker, locksmith).
- The flags-to-stored-formula seam is untested: replacing the sent record with
{...policy, allowedOrigins:['https://evil.example'], maxResponseBytes:1}keeps 29/29 green (prover).endo inspect <name> --jsonprints the stored record verbatim; onet.deepEqualon it pins the seam and simultaneously backs the "the daemon re-normalizes … agrees with this serialization" equivalence claim (http-mk.js:76-77). - The stderr effective-bound echo and the
tofu-autounbounded-grant warning are advertised legibility affordances, yet deleting bothprocess.stderr.writeblocks leaves the suite green (prover, corner-prober, locksmith). Add a daemon-driven assertion onresult.stderrusing a non-canonical input (e.g.--origin https://A.example/) so it also pins canonicalization, plus one on thetofu-auto+--acknowledge-unboundedwarning path. - The rebind tests assert only that pet names still appear in
endo list; a single-valued pet name means the assertion holds even ifmksilently returned the old client (prover).endo locate/inspect --jsondiscriminate (verified) — assert on the formula identifier. - The property test over
normalizeHttpClientOriginonly checks self-idempotence, strictly weaker than the "agrees with the daemon predicate for every accepted origin" claim the code makes; addt.is(new URL(o).origin, o)as the oracle (fast-checker). - The host-only confinement claim ("a guest cannot mint") is asserted in three places and pinned by none; add
mkguest gthenhttp mk c --origin … --as gasserting rejection (locksmith).
[rule: skills/regression-evidence/SKILL.md]
- The flags-to-stored-formula seam is untested: replacing the sent record with
-
The agent-facing primer hands an LLM the unbounded-mint recipe (locksmith).
packages/lal/primer/cli-reference.md:94-96is read by an agent and documents--policy-mode tofu-autotogether with the literal spelling of--acknowledge-unbounded— an operator-deliberation gate enforced only CLI-side, whose entire value is that a human pauses. Naming the token in the agent primer converts the pause into a copy-paste parameter an agent can hand a user. Fix: the primer should mentionstrictonly, describetofu-autoas requiring the human's own judgment, and defer the flag spelling toendo http mk --help. -
Design-doc synopsis omits a required flag and mis-cites the origin serializer (surfacer, packager, spec-keeper).
designs/cli-http-client.md§ Landed CLI surface renders[--policy-mode strict|tofu-auto] [--as <agent>], but two paragraphs later saysmkrefusestofu-autowithout--acknowledge-unbounded; the changeset andcli-reference.mdboth carry the correct[--policy-mode strict|tofu-auto [--acknowledge-unbounded]]. A reader copying the design synopsis gets a rejected command. Separately,http-mk-policy.js:21-22cites#concept-host-serializer, butURL.prototype.originis HTML's ASCII serialisation of an origin (url.spec.whatwg.org/#dom-url-origin) — the host serializer emits neither thescheme://prefix nor the default-port elision this module's normalization claim rests on. Fix: align the design synopsis with the changeset's, and correct the spec anchor.
Should-fix (strongly encouraged this round)
--origindocuments as required but renders as an ordinary option (surfacer):--helpshows[options] <name>and the requirement only surfaces as a thrown error; use Commander's.requiredOptionor annotate the help text.- The echo omits the caps it claims make confinement legible (assessor, locksmith, engine-realist, integrator): when
--max-requests-per-minute/--max-response-bytesare unset, the daemon defaults (60/min, 1 MiB) are frozen into the durable formula but never printed; with no Phase-1 inspect verb the operator can never learn the caps just minted. Echo both, marking defaulted values(default). Number.isSafeIntegeris the wrong ceiling for a byte cap (engine-realist):9007199254740991is admitted and durably frozen, butlimitResponseBytesaccumulates the whole body in the worker heap, so a 9 PB cap turns a large response into a heap abort /RangeErrorrather than atruncated()result.- Type divergence in a
checkJs:falsepackage (typist):makeHttpClientPolicy's@returnshand-rolls a partial that no declared type describes, while the daemon receiver requires all fourHttpClientPolicyfields; re-export the daemon types and@importthem. Two more type-narrowing findings (HTTP_POLICY_MODESinferringstring[], missing// @ts-checkonhttp-mk.js). - Duplicate origins baked into the persisted formula (assessor):
http-mk-policy.js:196maps without deduping;[...new Set(...)]preserves flag order. - Two naming corrections (stylist):
parsePositiveIntegerFlagis a factory named like a parser (renamemakePositiveIntegerFlagParser); test-localnamesholds output lines, not names.
Process
- No completion summary followed the round-5 responding pushes (scribe, summary-fix): round 5's only blocking item was a PR-body correction, which leaves no timeline trace — so absent a summary comment there is no record the blocking item was addressed, at what head, or with what verification. The fixer should post a round-5/6 completion summary (head SHA, per-item mapping, verification status).
Non-blocking (comment-only), for the fixer's awareness
- Dead-branch note: the
parsed.pathname !== ''disjunct is unreachable forhttp:/https:special schemes (removed in the surviving substance of the squash). - Property-test breadth:
arbAcceptedOriginusesfc.domain()only; a non-canonical IPv4-literal form (https://0177.0.0.1) exercises a different WHATWG host-parser branch and is untested (no bug suspected; coverage follow-up). - Coverage unverified: no c8 report at the worktree, so new-line coverage could not be confirmed (surfaced, not treated as covered).
Disposition: must-fix. Full per-seat aggregate retained in the panel run dir. This is one gauntlet stage: the fixer loop owns remediation; this review does not fix or un-draft.
model claude-opus-4-8 · harness claude · garden 33127fab
Apply panel round-6 items on packages/cli/src/http-mk-policy.js, replacing the round-5 red-then-reverted pair (219c440 harden-without-import, which reddened every plain-Node test leg, and afb9c2f reverting it) with one clean commit that carries the surviving substance plus the correct harden fix: - import harden from '@endo/harden' and harden(HTTP_ORIGIN_SCHEMES), harden(HTTP_POLICY_MODES). @endo/harden is already a runtime dep and its default export senses an existing lockdown-installed global and otherwise installs a local freezer, so it loads under the plain-Node unit test AND hardens under the shipped CLI (endo.js imports @endo/init). This matches the daemon's hardened twin (host.js:162-163) and AGENTS.md's harden-mandatory rule. - makeHttpClientPolicy now returns harden({ ... }): the record crosses E(agent).provideHttpClient and should be frozen at the mint site rather than trusting CapTP's callee-side freeze, per the in-package precedent (commands/form.js). - drop the unreachable `parsed.pathname !== ''` disjunct (the WHATWG serializer always yields at least `/` for the http:/https: special schemes) and correct the origin-serializer spec anchor: URL.prototype.origin yields the ASCII serialisation of an origin (scheme://host[:port]), not the bare host-serializer output the prior anchor cited.
…aces Apply panel round-6 documentation must-fixes: - designs/cli-http-client.md: the "Deferred, not carried" note claimed the #286 http-confine snapshot fix was irrelevant because exo-http-client "already snapshots headers and never sends the live Response across CapTP". Both halves are wrong: under lockdown, http-confine's request() returns freeze({ response, ... }) — deep-hardening the live undici Response independent of CapTP — and exo-http-client's headersToRecord then iterates the frozen Headers and throws. Rewrite the note as a Known Limitation: the Phase-1 client can mint and register but cannot complete a real outbound request on llm until the inert header-snapshot lands. Existing suites are green only because each injects a stub fetch whose headers is a plain record. - designs/cli-http-client.md: the Landed-CLI-surface synopsis omitted the --acknowledge-unbounded flag that mk refuses tofu-auto without; align it with the changeset/cli-reference form [--policy-mode strict|tofu-auto [--acknowledge-unbounded]]. - packages/lal/primer/cli-reference.md: the agent-facing primer named the literal --acknowledge-unbounded token, converting an operator-deliberation gate into a copy-paste parameter an agent can hand a user. Describe strict as the confining mode, describe tofu-auto as an unbounded grant needing the human's own judgment, and defer the flag spelling to `endo http mk --help`. - .changeset/cli-http-mk-phase-1.md: state the Phase-1 limitation for release readers (client registers and is inspectable but cannot yet complete a live request on llm).
…y guard Apply panel round-6 test-coverage must-fixes for security-relevant seams that were advertised but pinned by no test (deleting them kept the suite green): - The normalize idempotence property test only checked self-idempotence — true of any deterministic function. Add the stronger daemon oracle `t.is(new URL(o).origin, o)`: the exact predicate assertHttpClientOrigin applies (`new URL(o).origin === o`). - The effective-bound stderr echo is a Phase-1 legibility affordance (no inspect verb). Feed the registration test a NON-canonical origin (https://API.example.com:443/) and assert the echo carries the CANONICAL form, so deleting the echo or dropping canonicalization reddens. - The accepted-and-acknowledged tofu-auto mint's unbounded-grant warning path was pinned nowhere (only the REFUSAL was). Add a daemon-driven mint asserting both the minted-tofu-auto echo and the warning line. - The host-only confinement claim ("a guest cannot mint") was asserted in three prose sites and pinned by no test. Add mkguest + `http mk --as <guest>` asserting rejection and that no name is registered.
afb9c2f to
6ac7555
Compare
Fixer completion summary — panel round 6 (head
|
| Must-fix | Disposition |
|---|---|
| 1 — false #286 deferral rationale; client can't complete a request | Rewrote designs/cli-http-client.md "Deferred, not carried" as a Known Limitation: under lockdown http-confine's freeze({ response, … }) deep-hardens the live undici Response (independent of CapTP), and headersToRecord then throws iterating the frozen Headers. Took the design-correction path (not carrying #286 here) and stated the Phase-1 limitation in the changeset. |
| 2 — unhardened module constants on a false premise | import harden from '@endo/harden' (already an in-package dep/import); harden(HTTP_ORIGIN_SCHEMES), harden(HTTP_POLICY_MODES), deleted both "Not harden" comments; makeHttpClientPolicy now return harden({ … }). Verified the import loads and freezes under plain Node. |
| 3 — known-red intermediate commit | Squashed 219c440573+afb9c2fd95 into one clean commit carrying the surviving substance (dead-disjunct removal, WHATWG anchors) plus item 2's real harden fix. |
| 4 — advertised-but-unpinned security seams | Added: the daemon-oracle assertion new URL(o).origin === o on the normalize property test; a non-canonical-input stderr-echo assertion pinning canonicalization; a tofu-auto warning-path daemon mint; and a host-only mkguest + --as <guest> rejection test. (The inspect --json stored-record deepEqual — 4a — is left as a follow-up: its exact FormulaRecord serialization couldn't be confirmed in this worktree; see below.) |
| 5 — primer hands an agent the unbounded-mint recipe | cli-reference.md now names strict only, describes tofu-auto as an unbounded grant needing the human's own judgment, and defers the --acknowledge-unbounded spelling to endo http mk --help. |
| 6 — design synopsis omits a flag; wrong spec anchor | Design synopsis now shows [--policy-mode strict|tofu-auto [--acknowledge-unbounded]]; the origin-serializer anchor now cites URL.prototype.origin / the ASCII serialisation of an origin, not #concept-host-serializer. |
Follow-up (not blocking this stage): item 4a — pin the flags-to-stored-formula seam with endo inspect <name> --json t.deepEqual. The local install in this worker was wedged (shared yarn hardlink cache EMLINK), so the daemon-emitted FormulaRecord shape couldn't be confirmed to author that assertion without risking a red leg; it wants a follow-up with a live daemon. The should-fix items (.requiredOption for --origin, origin dedup, Number.isSafeInteger byte ceiling, type re-exports, naming) were not taken this round.
model claude-opus-4-8 · harness claude · garden 4e95097a
Refs: #286
Description
Adds the
endo httpsubcommand tree with its first verb,mk, which mints a confined outbound-HTTP client capability under a host-curated policy and registers it under a single pet name:This is the CLI surface stranded by #286, re-derived against the policy-based HTTP client that has since landed on
llm. #286 also carried a daemon-side reimplementation (a pairedhttp-controller+http-client,formulateHttpClient(allowedOrigins, ...),host.makeHttpClient(controllerName, clientName, allowedOrigins)) that hard-collides with what landed (@endo/exo-http-client'smakeHttpClientAndControl,host.provideHttpClient(name, policy)), so a rebase of #286 was aborted. This PR takes only the CLI-layer value and drops the daemon reimplementation, mapping--originontopolicy.allowedOriginsand the guard knobs onto the optional policy fields. The maintainer approval on #286 (2026-08-15) predates recognition of that collision and does not carry here.Most-critical files to review:
packages/cli/src/commands/http-mk.js(the verb),packages/cli/src/http-mk-policy.js(the pure flag→policy assembler and local flag validators), andpackages/cli/src/endo.js(thehttp mkwiring).Security Considerations
The verb introduces a new outbound-network authority: a minted client can issue HTTP(S) requests to the origins its policy admits. That authority is confined by the daemon's
@endo/http-confine/@endo/exo-http-clientlayer, not by any new CLI-side check — the SSRF-by-redirect defense (redirect: 'manual'plus per-hop origin re-check), the per-response byte cap, and the sliding-window rate limit all live one layer down, andmkonly assembles and forwards the policy record.The confinement bound depends on
--policy-mode:strict(the default) confines the client to the listed--originset.tofu-autoauto-allows any first-seen origin, so--originbecomes a pre-seed rather than a bound and the allowlist no longer limits outbound reach (atofu-autoclient can reach loopback/link-local targets such as a cloud metadata endpoint). Phase 1 ships noinspect/revokeverb, so an operator cannot yet see what got auto-pinned or undo it. Because that grant is unbounded and unrevocable,mkrefuses--policy-mode tofu-autounless--acknowledge-unboundedis also passed (a local reject, by flag name); on a successfultofu-automint the verb also warns on stderr. Preferstrictuntil the inspection/revocation verbs land.--asnames a host —provideHttpClientis a host-only method, so a guest cannot mint a client. Re-runningmkon an existing name rebinds the name to a new client under the new policy; the previous client's name reference is dropped, and the daemon'sstoreIdentifier→removeEdgeIfUnreferencedcollects the orphaned client unless another edge still retains it (for example it was granted to a guest under another name), in which case it survives the rebind and revocation awaits a later verb. Flag values are validated locally (origin shape/scheme, positive-integer guards, admissible mode) so a lexical error is reported by flag name before anything crosses CapTP; the daemon'snormalizeHttpClientPolicyremains the authority on policy semantics. On success the verb echoes the canonical origin allowlist and policy mode on stderr, so the confinement is legible without aninspectverb.Scaling Considerations
None. The verb mints one capability per invocation. The client's own resource use is bounded by the rate and byte caps in its policy (defaulting to 60 requests/minute and a 1 MiB response cap).
Documentation Considerations
The new verb group is documented in
designs/cli-http-client.md§ Landed CLI surface (Phase 1), in the@endo/clichangeset, and inpackages/lal/primer/cli-reference.md(the CLI reference the agent stack reads) under a new Network group. No backwards-compatibility or upgrade concern: the change is purely additive.Testing Considerations
packages/cli/test/http-mk-command.test.jscovers the pure policy assembler (record shape, omitted-when-unset knobs, origin order/arity, browser-copied-origin normalization throughmakeHttpClientPolicy— the seam the user reaches — empty-allowlist rejection, non-http scheme rejection), the local numeric-flag and--policy-modevalidation (including theNumber.isSafeIntegerboundary and the--acknowledge-unboundedgate ontofu-auto), the--helpsurface (including thetofu-autowidening wording), and daemon-driven registration and rebind round trips. Property-based coverage of the assembler landed with this PR:@fast-check/ava(acatalog:devdevDependency, oneyarn.lockline) drives an origin-normalization idempotence property and a reject-partition property over the path/query/fragment/userinfo suffixes. The daemon-side origin-acceptance corner is closed by construction rather than deferred — both layers computenew URL(x).origin, so the CLI's output is the daemon's verbatim fixed point.Compatibility Considerations
Additive; no prior usage pattern changes.
Upgrade Considerations
None — no persisted-data or production-system migration.
Out of scope
allow/deny/revoke/inspect) land in later phases; onlymkships here.http-confineinert-response-snapshot change is assessed but not carried: it is not onllm, its relevance to thellmcode path is unverified, and — the reason deferring it is safe here — no test in this PR dials out through the confined client (the new tests mint a client but never issue a request), so theharden(Headers)/feat(daemon,cli): endo http mk Phase 1 (controller + client cap pair, designs/cli-http-client.md) #286 crash path is never reached regardless of Node version. CI matrixes both Node 22.x and 24.18.0 (there is no Node-24-only pin), andpackages/clisupports^20.17.0 || >=22.9.0, so Node 22 is exercised — the crash simply is not on any path this PR's tests take. It should land separately on its own merits with a Node-22 reproduction. The recommendation is recorded durably indesigns/cli-http-client.md§ Landed CLI surface (Phase 1) (see "Deferred, not carried"), not only in this description.