Review finding, valid: under the registry's Code Mode the assembly's
only wire tool is run_code, which the plan allowlist filtered out —
leaving the model with NO tools at all, the exit review included. The
composition exists today (the acp-agent example ships a code-mode
overlay), so plan mode bricked it outright.
run_code is a transport, not a capability: every bridged sub-call is
serialized back through ToolRegistry.execute() carrying the same agent,
so tools/pre-execute judges each capability individually — exactly like
native calls. Both layers now exempt it by name: the filter keeps it
visible (tests pin plan-mode Code Mode assembly = ['run_code']) and the
gate passes the wrapper while the same run's write sub-call still
denies with the plan-mode reason.
Documented residual, same class as the prepend-after-load one: the SDK
section renders from the registry's store, so a plan-mode program may
be offered bindings whose dispatch the gate then denies — nothing runs
that a native call could not.
Review finding at the seam of two surfaces this branch added to the
same stdin: with an ask_user_question (or plan-review) prompt active,
the line handler dispatched every line as the answer first, so
'/mode plan' typed mid-question was recorded as free-text feedback —
model-visible in the tool result — and the mode never changed. Command
handling now runs before answer dispatch: the command executes, the
question stays pending and still owns the next non-command line. A
literal '/mode…' free-text answer is the trade-off deliberately spent —
a swallowed command that becomes review feedback costs far more than
that contrived answer shape.
Review finding with a real in-repo instance: the structured runtime's
per-spawn final-assembly wrapper (prepend, post-next) re-injects
structured_output OUTSIDE the mode filter, so a structured child in
plan mode would see a tool the gate then denies — the soft policy and
the hard gate telling different stories. The suggested fix (make the
mode filter outermost) cannot beat that instance: prepend unshifts, so
the per-spawn listener always registers later and wraps outer.
Two-part resolution instead. Semantically, structured_output enters the
shipped plan allowlist — it is a child's pure result channel, the same
ask/report class as ask_user_question and exit_plan_mode, so the
filter, the re-injection, and the gate now agree wherever a structured
child runs in plan mode. Mechanically, the filter registers with
prepend anyway: it now wraps outside every append-registered listener
regardless of load order (regression test pins a pre-registered
post-next mutator being filtered), narrowing the documented cosmetic
residual to prepend-after-load listeners only, where the gate still
covers execution. Severity note: no execution breach existed — the gate
held throughout; this closes the prompt-honesty gap.
Live-session feedback (a real Zed elicitation round-trip): the model
presented its finished plan as a plain reply and asked the USER to
switch modes — the exact reversal the roadmap warns about — because the
shipped section's 'present it with the exit_plan_mode tool' read as a
suggestion. The section now says a finished plan is delivered by
calling exit_plan_mode, preferred over pasting it as a plain reply or
asking the user to switch modes — firmer, without imperatives.
ask_user_question enters the shipped plan allowlist (asking is
read-only-safe), and the section points a blocked decision at it. The
plan-acp-agent example composes the bash family (default mode only —
plan's allowlist keeps excluding it, so the two modes now demo a real
difference) plus tool-ask-user; both recorded scenarios re-recorded:
the pin now shows plan = [ask_user_question, exit_plan_mode, read,
todo_write] and post-exit default = the full eight-tool surface.
Review finding: the exit tool's direct mode/set append flipped the
folded mode while the loop could still execute further tool calls from
the SAME assistant response — a same-batch exit_plan_mode + write pair
would sail past tools/pre-execute under 'default' even though the
request was assembled under the plan-shaped header. That broke the
design's own invariant (a step's executions run under the mode its
assembly folded), which the pending-intent flush was built to hold for
user flips.
The tool now records the switch as a pending intent like every other
writer, flushed at this step's end (still in-turn); pending intents
carry a narrate flag so the exit's flush stays silent — the tool result
is its narration — while user flips keep the coalesced boundary notice.
The gate, folding the logged mode only, now provably covers the whole
batch: regression test pins approve-then-write-in-the-same-batch as
denied, and the widened toolset still arrives on the next step.
The recorded scenarios are re-recorded: the fixture now shows mode/set
landing after step/end, before the widened fallback header.
The with-key recording session the RFC deferred. plan-mode is the 'plan'
header class's pinning scenario and necessarily carries BOTH header
shapes verbatim: the plan-shaped initial snapshot and the widened
fallback snapshot the approved exit produces mid-turn — so the suite
factory's pin guards relax from exactly-one to at-least-one header (the
classmates' uniformity anchor is the pin's FIRST header; transition
headers are legal only in the pin, matching the sandbox stack's
precedent). plan-mode-reject pins the keep-planning branch: the
corrective isError carries the reviewer's free-text feedback verbatim
and the session stays in plan mode, one header, uniform with the pin.
Recording notes, encoded in the prompts: the model is pinned to RELATIVE
paths (a recorded absolute temp path neither replays on another host nor
normalizes — the normalizers scrub the run's own cwd, not the
recording's), and the recorded model never calls a filtered tool, so the
gate's deny path stays pinned at the unit tier — that refusal is the
behavior the soft layer exists to produce. The gen-tool-catalog
meta-test pins exit_plan_mode in the harvested schema list.
The proposal survives contact with the code with three amendments, per
the RFCs-are-proposals rule. (1) A mode transition logs a
request/header-delta only when expressible: adding exit_plan_mode
resorts the canonical tool list, and a pure reordering has no delta
form, so entering plan mode logs the full fallback snapshot — the
attributability claim holds either way. (2) The proposed/ skeleton
converts to the implemented grammar: Proposal → Decision, the roadmap's
staging (now history) drops to the standing Deferred list, and
Acceptance criteria + Risks fold into Consequences (what holds, then the
accepted costs, including the ACP v2 mode-removal migration). (3) The
two recorded scenarios stay pending a with-key session, recorded in
Deferred.
The docs tail completes: the cookbook's plan-mode row upgrades from
sketch to the shipped package, architecture.md gains the ctx.modes
capability row (ceiling 1640 → 1650: a new capability service's table
row does not fit the old budget), and every reference repoints to
implemented/.
The sandbox execute wrapper JSON round-tripped the return and blindly cast it
to ToolExecuteReturn. A JSON-valid but wrong-shape return — a bare string,
{ content: 'ok' }, blocks without a type tag — sailed through: the registry
spreads result.content, so { content: 'ok' } became ['o','k'], passed the
session log's isJsonValue gate, and the DeepSeek serializer then flattened it
to '(no output)' — silent corruption of the next model request and every
replay, instead of a contained tool error.
The round-tripped value is now shape-checked against the two ToolExecuteReturn
forms (array of content blocks, or { content: blocks, meta? }); block checks
are structural only (plain object + string type tag) because the ContentBlock
union is merge-extensible. A wrong shape — and the formerly cryptic
forgot-return/bare-string cases — fails that one call with a teaching error
echoing a truncated preview of what was returned and the two valid forms.
New specs pin the object-form pass-through (meta included), six rejection
shapes, and the preview truncation; per-file 100% coverage holds.
The sandbox docs overclaimed a containment contract the design never makes:
"capability access is routed through cordis services, never Node built-ins,
so everything a mounted plugin does stays inspectable and disposable". The
host-realm helpers on the sandbox global (harness, console, btoa) are
reachable functions, so mount code that goes looking can reach the host realm
through one of them — accepted under the trust stance, because the ctx a
mount ultimately receives is fully privileged anyway. Reword the sandbox
module doc, the README trust stance, and the RFC sandbox-semantics section to
say exactly that: the traps and small global surface STEER honest code onto
the cordis services; they are not a security boundary.
Two review findings (#220) on the sandbox context façade:
- Undeclared services were reachable: the façade resolved any live global via
ctx.get(name), so ctx.bash worked without inject: ['bash']. A cross-mount
consumer could then depend on a provider cordis never saw — unmounting the
provider would neither park the consumer nor unwind its registered tools,
leaving a model-visible tool that fails only at execution. The façade now
reads ctx.fiber.inject and refuses any service the mount did not declare
(with a teaching error naming the inject fix), so the dependency is always
visible to cordis and its activation/unload semantics bind.
- ctx.tools.get returned the live ToolDefinition, including execute — mount
code could call another tool directly and bypass ToolRegistry.execute and
its pre/post-execute hooks and accounting. get now returns the same
read-only name/description/parameters view as schemas(), never an invocable.
Adds inject-gate and schema-view regression cases to sandbox-context.spec.ts
(undeclared property/get denied, declared allowed, the cross-mount zombie-tool
scenario refused at call time, get exposes no execute). Package stays at
per-file 100% coverage. RFC, mount description, and tool-catalog updated.
Review finding (#220): the guarded proxy only special-cased ctx.tools, so
mount code could reach an UNGUARDED context through ctx.root, ctx.extend(), or
a service instance's .ctx, then ctx.root.tools.register({…}) to bypass the
marker check and host-realm normalization — a raw vm-realm result would later
error a real agent turn at the session-log plainness check.
The sandbox ctx is now a whitelist façade, not a pass-through proxy: it exposes
only what a mount needs — tools.register (marker-guarded), on/once, provide, the
timer helpers, and injected services resolved through a guarded get — and denies
every framework-plumbing member (root, parent, fiber, reflect, registry, extend,
isolate, intercept, plugin, set, mixin, …) with a teaching error. Injected
services are wrapped so a method returning a Context is rejected on the way back
(the .ctx escape), closing the one indirect leak. There is no context-valued
member left to reach; cross-mount provide/inject is untouched (the plugin's own
inject and the fiber's pending/active gating are unchanged). ctx.plugin (child
plugins) and ctx.set are denied by design; ctx.effect is deferred (FIXME).
Adds tests/sandbox-context.spec.ts covering the escape class (root/extend/fiber/
plugin/set/… denied, the classic root.tools.register bypass, the .ctx escape,
read-only writes) plus the async-service and symbol/in-operator paths for 100%
coverage. RFC/README/tool-catalog/config-catalog updated; api-catalog.ts
regenerated (also picks up the codeRuntime service that entered on the master
merge and was left stale).
The tree SHAPE was the wrong surface for the model: what it needs from
cordis_inspect is what services, plugins, and capabilities are loaded, not the
fiber hierarchy. The plugins section is now a flat name + lifecycle-state list
from ctx.registry (deterministically sorted, one line per instance); the ASCII
tree renderer, the parent→child rebuild, and the dyn-id tree annotation are
deleted — dynamic mounts keep their own richer dynamic section (id, state,
provides, waits). Net -49 lines; RFC and READMEs state the flat-list contract.
Field sessions showed models writing tool schemas in the JSON-Schema dialect
by strong prior — type: 'integer', required: false, then the full
{ type:'object', properties, required: [...] } wrapper — and the rejection
text itself pushed a nearly-correct DSL attempt BACK to raw JSON Schema: one
stats tool cost three consecutive schema errors before mounting. The boundary
now normalizes wherever the input has exactly one meaning (wrapper unwrapped
with the required array becoming per-property flags at any nesting level,
integer → number, required: false → optional, all rebuilt as fresh host-realm
objects) and rejects only genuinely meaningless input, enumerating the valid
vocabulary in the error. Re-running the failing session mounts first-try.
The mount description documents both accepted forms.
The design record for tool-cordis: the three-tool contract, the vm sandbox
trust stance and boundary mechanisms, the dynamic-group lifecycle, cross-mount
provide/inject composition, the generated runtime API catalog, and the
alternatives weighed (per-capability registration tools, hand-maintained API
tables, a mount provenance event, a hardened sandbox).
Both demo:code-mode UIs now share one mechanism: the base example plus a
same-shaped code-mode.cordis.yml include overlay (insert the worker
runtime, flip tools.mode). Previously the REPL side was a hand-forked
example (examples/code-agent) that had also silently diverged — it
dropped compaction and the subagent stack — so the demo's UI argument
switched agents, not just surfaces. The fork is retired: coding-agent
gains the overlay, a Code Mode README section absorbing code-agent's,
and both of its tests (the keyless boot guard, retargeted at the
overlay; the with-key RFC proof, which hand-mounts its own harness and
moves untouched). The RFC's composed-surface and e2e-tier lines, the
examples index, the AGENTS.md smoke table, and the dsh-tools README
link now describe the overlay shape.
Verified live: overlay keyless smoke, with-key code-mode e2e from its
new home, demo:code-mode banner + EOF exit, and the acp handshake.
Code Mode is the point; the UI is just the surface it happens to wear.
demo:code and demo:acp-code collapse into one dispatcher
(scripts/demo-code-mode.mjs): `pnpm run demo:code-mode [repl|acp]` —
repl (default) boots the stdio REPL over examples/code-agent, acp
serves examples/acp-agent's code-mode overlay; each UI runs the exact
node invocation its standalone script ran, and an unknown argument
fails loud with usage. All nine references across READMEs, the RFC,
the overlay header, and the keyless-smoke comment renamed. Smoked all
three paths: usage exit 2, ACP initialize handshake, REPL boot + EOF.
The seam shipped without its own RFC — the reconstructable-requests RFC
was amended with the mechanics, but the decision record (why a
compose-once frozen prefix, and what the per-request before/after shape,
a system-prompt section, a durable history opener, per-turn composition,
and a dedicated session event each lost to) had no home. Implemented
lifecycle, feature class, dated to the first commit of the work.
The dsh-tools half of the Code Mode RFC (its fourth, final change): the
registry gains its first config — mode: native | code | both — and OWNS how
its tools reach the model. 'code' contributes exactly one wire tool,
run_code, plus a lazy tools:sdk prompt section declaring every other tool
as a generated TypeScript API (jsonSchemaToTs: total over the defineTool
subset, unknown degradation, lexicographic byte-identical rendering);
'both' ships both representations; 'native' is byte-for-byte the old
behavior. Non-native modes fail every assembly loudly without a
typescript-language ctx.codeRuntime.
run_code's dispatch bridge: JSON-normalizes each binding argument before
dispatch (what dispatches is what the tool/code-dispatch event logs — the
append can never fail on payload shape; BigInt/circulars reject that one
call), serializes all program tool calls through a per-run queue (even
Promise.all — no concurrency-safety metadata yet), routes every sub-call
through tools/pre-execute → tools/post-execute (a deny rejects the
program-side promise), drops sub-call additionalContext (no safe outlet
mid-run; pinned), owns a run-scoped abort that follows the outer signal in
and fires on settlement (in-flight sub-dispatch aborted, queued abandoned,
queue drained before returning), and converts a failed run into
CodeRunFailedError → a structured isError carrying kind + captured logs.
tool/code-dispatch joins SessionEventMap by declaration merging (log-only;
deriveMessages ignores it).
The composed surface: the tools config forwards through agent-core and
both app packages; examples/code-agent + demo:code run the worker runtime
under mode code (keyless boot smoke + a with-key e2e proving the collapsed
[run_code] header, the dispatch events, and the file the program wrote);
two new snapshot scenarios (code-mode-turn, both-mode-turn) record the SDK
section, collapsed header, dispatch events, and result card — each its own
header-pinning class (the harness gains per-scenario config overlays and
per-class pins). Catalogs, graphs, cookbook, hooks-bridge notes, and the
RFC (moved to implemented/, restructured to decision-era headings) updated
in the same change.
Review follow-up (#196): a listed name with no registered tool was silently
ignored; misconfiguration must block work instead. The check lives in the
assembly — the earliest moment the registered tool set exists (tool plugins
register after the service constructs) and the only universal one (cordis has
no "all plugins loaded" event; registrations change at any time). assemble()
is now async so the throw surfaces as a rejection rather than a synchronous
escape from a Promise-returning method.
Blast radius, pinned by a loop-level test: the rejection reaches the turn's
outer catch — the turn closes balanced with an `error` reason, agent/error
mirrors it, no step opens, no request/header is logged, no request reaches
the adapter, and the agent returns to idle; every turn fails identically
until the config is fixed. A boot-time validation pass was considered and
rejected (recorded in the RFC). The general principle — misconfiguration
fails loud, never a silent skip — is added to AGENTS.md.
A three-dot rest entry reads as elision in a cordis.yml; the spelled-out
sentinel says what lands there. The literal now appears once in code (the
constant) and once in the value-pinning test; every other reference — the
forwarding tests included — imports TOOL_ORDER_REST, which adds the
dsh-system-prompt devDependency to the two app packages.
Review follow-up on #196.
Define the in-file RFC contract in docs/rfc/README.md § The file format:
the header block (`# RFC: <title>` plus a dateless Status enum
cross-checked against the lifecycle folder), the per-lifecycle body
skeleton (a Problem opener everywhere; Proposal/Alternatives considered/
Acceptance criteria/Risks in proposed/; present-tense Decision/
Consequences with proposal-era headings banned in implemented/; the
frozen proposal shape in rejected/), and a mandatory Alternatives
considered section with a date-fenced grandfather comment for pre-format
RFCs whose alternatives are not reconstructible from the record.
Enforce it with a new doc-sync gate, scripts/verify-rfc-format.ts, and
normalize all 112 RFCs to it: ~15 Status-line spellings collapse to the
enum, 29 Context openers become Problem, the 39 legacy-format XXX debt
markers are resolved and banned from reappearing, proposal-era sections
in implemented RFCs are rewritten to shipped reality (including the
web/fs/subagent seam RFCs' migration plans and test checklists, closing
the doc-tiers deferred-work item on the web seam), every RFC gains an
Alternatives considered section or the grandfather comment, and the
bilingual pair is re-mirrored and re-recorded.
Move the generated index tables out of README.md into a fully generated
docs/rfc/INDEX.md — gen-rfc-index now writes the whole file, and
verify-rfc-classification checks its freshness and rejects index-shaped
rows in the curated README — which makes room for the format contract to
live in the README front door instead of a separate FORMAT.md.
The decision record, and the first RFC written in the new format, is
docs/rfc/implemented/process/2026-07-05-uniform-rfc-format.md.
Master's docs-overhaul stack (#142-#144) rewrote AGENTS.md into the
slim budget-gated form and repointed the review skill's citations.
Resolutions:
- AGENTS.md: master's rewrite wins; the no-hardcoded-tunables
convention is re-added as one terse bullet in the new style, after
'Explicit > implicit at package seams'. Within the verify-doc-budgets
ceiling, so no displacement or raise needed.
- dsh-code-review SKILL.md: master's repointed citations win; the
hardcoded-tunables reviewer check and the Conventions keyword are
re-applied on top.
- packages/README.md: master replaced the hand-maintained dependency
list (which carried this branch's chars-per-token wording) with a
pointer to the generated module graph — master's side taken whole;
the estimator wording lives on in the compact package READMEs.
A Codex review pass on the draft caught four real gaps and two solid
suggestions; all addressed except one pushed back on the merits:
- hooks-claude/hooks-codex: stderrSummaryMaxChars was the one new knob
with NO range validation — a negative/NaN cap would silently
misbehave inside slice(). Both bridges now assert a positive integer
at the TOP of apply() (before the config-file parse's early return,
so a bad value fails the load loudly), with rejection tests.
- tool-fs: the read caps count lines/chars/bytes, so positive-FINITE
was too loose (a fractional readLimit would flow into windowing
arithmetic and the schema description). All four now require a
positive integer, matching tool-web's cap.
- Doc drift the gates cannot catch: tool-web's README tools table
still named WEB_SEARCH_MAX_RESULTS as the mechanism; compact-basic's
README/module doc and the compaction-capability-seam RFC still
described estimation as fixed char/4 rather than the charsPerToken
default.
- subagent-acp: the dispose graces were tested only at the
startAcpRun level, so a regression that stopped threading plugin
config into AcpRunSpec would have survived. A provider-path test now
drives the trap-escalation scenario through ctx.subagents.start with
small config graces and bounds dispose at 4s.
Pushed back on: converting compact-basic's charsPerToken to a
schemastery field. The package's whole config is deliberately
hand-rolled (resolveConfig, every threshold REQUIRED with no default —
a documented design posture); one schemastery field beside it would be
incoherent. The knob is cordis.yml-reachable, defaulted, and validated,
which is what the convention requires; migrating the package to
schemastery wholesale is pre-existing config-surface hygiene out of
this change's scope.
The Execution bullet cited "the bash-seam PR" — a change unit a reader of
the current tree cannot see. State the standing fact instead, matching the
runner module doc's own phrasing.
