Skip to content

feat(sandbox): run Routers and Handlers in the subprocess sandbox by default (BACKLOG #1278) - #879

Open
wshallwshall wants to merge 5 commits into
mainfrom
claude/backlog-1278-sandbox-default
Open

feat(sandbox): run Routers and Handlers in the subprocess sandbox by default (BACKLOG #1278)#879
wshallwshall wants to merge 5 commits into
mainfrom
claude/backlog-1278-sandbox-default

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

Closes the change named in BACKLOG #1278: [sandbox].mode now defaults to "subprocess", so a
stock serve runs Router and Handler bodies in a per-inbound worker child instead of the engine's own
address space. The isolation mode has been built and exercised since ADR 0087. Only the default was
wrong.

The item stays open for two vault-side rows a Builder cannot reach. See the last section.

First, the thing this does NOT do

It does not stop config Python executing in the engine process. The loader still runs every *.py
in the config directory in-process, as the service account, at every serve and every reload, ungated
by mode. What mode governs is where a Router's or Handler's body runs once the graph is built.
Module top level is out of its reach either way, and the safe-source DACL gate is still what covers it.

This is stated first in the setting docstring, in pipeline/sandbox.py, in CONFIGURATION.md and in
DEPLOYMENT.md, because a reader must not buy a boundary that is not there.

The samples were RUN, not inspected

The item warns in capitals that the shipped samples look compatible and that this is not the same as
having run them. So they were run, as a paired A/B on the same tree:

bare default, no [sandbox] section at all mode=off control
inbounds listening 9 of 13 (7 MLLP, 2 X12) same 9
MLLP ACKs 7 of 7 AA same
X12 interchanges accepted 2 of 2 same
File pollers (./in/fhir, ./dev-inbox/pdf) both files consumed same
end-to-end delivered to live receivers OB_ACME_ADT 2, OB_EPIC_STREAM_MDM 1 identical
_sandbox_worker children 11, one per inbound with traffic 0
SandboxError occurrences 0 0

Same ACKs, same deliveries, isolation on or off — so the instrument discriminates rather than merely
reporting a green. Delivery was measured against real MLLP receivers I stood up on 2601/2641/2776,
so this is the full path (ingress, sandboxed Router, sandboxed transform, outbound delivery, downstream
ACK), not just the inbound ACK.

The winning arm declared no [sandbox] section whatsoever, which is what proves the default alone
carries it.

All 14 modules under samples/config/ loaded — 13 inbound and 12 outbound connections constructed —
and 11 inbounds executed Router and Handler bodies inside a worker child. Not exercised: the DICOM
C-STORE SCP on 11112 (needs an SCU) and the IB_RTE_RESPONSE loopback.

A counting trap, so nobody re-derives a phantom 2x

The raw process count was 22, not 11, and it looked exactly like workers spawning workers — which
would have contradicted the item's finding 2. It is not. On this box
.venv\Scripts\python.exe is a redirector that re-executes with a byte-identical command line, so
every logical Python process appears twice in Win32_Process under the same CommandLine. Proven
on the parent: one serve launch produced two processes with identical command lines, and
sys._base_executable differs from sys.executable. The real figure is one worker per inbound with
traffic, exactly as filed.

Two failures that are not mine

OB_IMMUNIZATION_BODYCRED and OB_IMMUNIZATION_REGISTRY fail to start because environments/dev.toml
carries none of the registry_* values. They appear identically in both arms, including the
mode=off control, which is the proof they are pre-existing. The engine isolates them and continues.

The smoke also needs [security].handles_real_patient_data = false, or serve --env dev refuses to
start without a store encryption key.

The capability the default costs you

db_lookup and fhir_lookup are fail-closed REFUSED inside the child. They re-enter the event
loop, which a process boundary breaks. A Handler needing either must run mode="off". That escape
is retained, supported, and tested here; it is not going away.

mode is read once at engine construction, so /config/reload does not re-read it and changing it
needs a restart.

The five findings the item named

Honoured, not re-derived. ADR 0087's numbers are cited, never re-measured.

  1. The flip arms a 5-second wall cap that nothing enforced before. At mode=off run_sandboxed is
    literally fn(payload) with no timeout at all. Documented both ways round: a busy loop can no
    longer wedge intake, and a legitimately slow Handler now dead-letters where it used to finish.
    test_threat_model_doc_drift.py's label read "no Router/Handler wall cap", which the flip made
    backwards; corrected.
  2. Per-inbound resource multiplier — one child process, two parent daemon threads, three parent
    pipe fds, and on Windows a job-object handle, per inbound with traffic. Now in CONFIGURATION.md.
  3. One message is not one dispatch — three dispatches for a single handler with an accepts=
    predicate, 1 + 2K for fan-out to K. In the table beside the 0.19 ms / 6.2 ms figures, so the
    headline number cannot be read without its multiplier.
  4. fuse_thread_hops now carries the warning that the runner hard-disables fusion underneath it
    whenever the sandbox is on. An operator reads the knob they set, not the other one.
  5. Gate fidelity — DECIDED, not deferred. The item offered a choice; I took "say plainly that it
    does not." dry_run() takes no sandbox argument and always runs in-process, so a Handler calling
    db_lookup/fhir_lookup passes messagefoundry check green and then fails closed at serve,
    and wall_seconds is unenforced in the preview. Both dryrun.py and checks.py now say so
    outright. Teaching the gate to spawn a worker per inbound is its own change with its own cost;
    route_only/transform_one already take sandbox=, so the seam is there when someone wants it.
    Note checks.py's existing "previews what the default engine actually delivers" claim is scoped to
    snapshot_on_send and stays true — but it generalizes badly, so the limit sits right beside it.

The two places the old default was load-bearing

ADR 0144's rejected-alternatives rationale — the stated reason the static lint gate exists at all —
gave two grounds for refusing to rely on the sandbox alone, and "it is opt-in/off-by-default" was one
of them. That ground is gone. The rejection survives on the other, which is sufficient alone: an
address-space boundary does not catch a Handler leaking PHI into the store's own log or building SQL
inside the sanctioned db_lookup, because neither crosses an address space. Written so a reader can
see the gate still has a reason to exist.

docs/ASVS-ASSESSMENT-METHOD.md:115 used this exact cell as its worked example of rule 5, "a
working control that ships off"
. That premise expired. The fact is corrected; the partial verdict
is deliberately left standing
— re-scoring a cell is the tracking seat's act against the vault, not a
Builder's. The table now says plainly that no live worked example of rule 5 remains, rather than
inventing a substitute, which is the choice its own 3.7.3 row already made.

Also swept

config/settings.py, pipeline/sandbox.py (module docstring, SandboxMode.OFF), pipeline/engine.py,
pipeline/wiring_runner.py, checks.py, pipeline/dryrun.py, docs/CONFIGURATION.md,
docs/DEPLOYMENT.md, docs/PHI.md, docs/adr/README.md, ADR 0087 (amended, not rewritten),
ADR 0144, ADR 0147, tests/test_threat_model_doc_drift.py, tests/test_phi_logging_inventory.py.

SandboxPolicy.mode lost its = SandboxMode.OFF — a second default free to contradict the first. All
12 construction sites already passed mode= explicitly, so requiring it was free.

tests/test_settings.py needed no edit: its round-trip arm already picks the opposite of whatever the
current default is.

Checks run vs skipped

Interpreter provenance, since a green from the primary checkout would say nothing about this change.
Run with C:\Users\Scott\Code\MessageFoundry\.venv\Scripts\python.exe from this worktree as cwd, and
verified before trusting any result:

messagefoundry.__file__ =
  C:\Users\Scott\Code\MessageFoundry\.claude\worktrees\agent-a5d66744bbc7c96bd\messagefoundry\__init__.py

That path is inside this worktree, so the tests imported the code under review.

Ran, green:

Leg Result
test_sandbox.py 35 passed (66 s)
test_sandbox.py + _codec + _import_boundary 110 passed (81 s)
test_threat_model_doc_drift.py, test_phi_logging_inventory.py, test_settings.py, test_accepts_seam.py 192 passed, 89 skipped
test_checks.py, test_checks_gate_parity.py, test_dryrun.py, test_dryrun_snapshot_parity.py 96 passed, 1 skipped
test_doc_guards_lane.py, test_doc_ref_handle.py, test_docs_cite_no_refused_config_keys.py, test_adr_analyze.py, test_docs_security_pathways.py 323 passed
ruff check . / ruff format --check . clean, 1241 files
mypy messagefoundry (strict) clean, 267 source files
scripts/docs/backlog_status_check.py OK, 664 items, each declaring exactly one status

Deliberately skipped — CI's job. The full engine suite. This is not a judgment call I made
lightly: the item's own dispatch note demands a full-suite lane, and the previous lane reverted this
exact change for want of one. The box is shared and the suite is not merely slow here, it is
unavailable — a peer measured 4 percent in 18 minutes against 28 concurrent pytest processes on 20
cores. Running it under saturation would measure the contention, not the flip. Please read the full
suite on the hosted runner before merging
, along with windows-service-smoke, which never runs
locally.

Two known-spurious failures, reported by the Lander today. If tests/test_worktree_gate_control_plane.py
reds, it is not from this branch — it is failing across unrelated PRs that do not touch it while main
is green. If the web console leg on windows-2025 reds with 405 passed and zero failures, that is a
timing gate rather than a real failure.

The doc-content half of test_threat_model_doc_drift.py is inert here and in CI (the vault document
is absent from every public checkout), and it emits a ThreatModelDocUnenforced warning saying so. That
is expected, not new.

What keeps the item open

Two coupled rows in the vault-only docs/security/THREAT-MODEL.md, which no checkout can see:

  1. The 15.1.3 row must be rewritten to say the Router/Handler wall cap is enforced on the
    shipped default, not off it.
  2. _DANGEROUS_ROW_KEYS still anchors on "**In-process (default) or subprocess-isolated execution",
    which now names false vault text. The anchor is deliberately NOT re-picked. Choosing a
    replacement from a checkout that cannot read the document would trade a stale anchor for an
    unverifiable one and red the leg for whoever does hold the vault.

Both are marked in the test file as coupled vault-side edits, the way the pickle anchor beside them
already is.

Re-scoring signal

The prior lane's read is confirmed: difficulty 3 was wrong, 5-6 is right. The change is one default;
the verification is a full suite this box cannot run, a sweep across roughly 14 sites in code, tests and
six documents, plus two vault documents, and five findings each needing release-note prose the row did
not budget for. Value 6 still looks right.

Zero deployments (CLAUDE.md section 0), so this cost no migration and there was nobody to notify. That
removed the vacuous cost of a staged rollout; it did not lower the bar on proving the new default works,
which is what the paired A/B above is for.

🤖 Generated with Claude Code

wshallwshall and others added 5 commits September 4, 2026 17:30
…default (BACKLOG #1278)

`[sandbox].mode` shipped "off", so on the shipped default a Router or Handler
body ran in the engine's own address space, next to the DEK, the audit chain and
every connection's in-flight data. The isolation mode has been built and
exercised since ADR 0087. Only the default was wrong. This flips it to
"subprocess".

State the limit first, because a reader must not buy a boundary that is not
there: this does NOT stop config Python executing in the engine process. The
loader still runs every *.py in the config directory in-process, as the service
account, at every serve and every reload, ungated by mode. What mode governs is
where a Router's or Handler's body runs once the graph is built.

mode=off is untouched and stays supported. It is the escape for a Handler that
needs the live db_lookup/fhir_lookup bridges, which fail closed inside the child
because they re-enter the event loop.

The docstrings now carry the costs ADR 0087 already measured, rather than
re-deriving them: the refusal of live enrichment, the wall cap that starts being
enforced (there is no timeout at all at mode=off), the per-dispatch throughput
and the fact that one message is not one dispatch, the per-inbound process and
handle multiplier, and the pre-deploy gate's blindness to the setting.

SandboxPolicy.mode loses its `= SandboxMode.OFF`. A second default on the
dataclass would have been free to contradict the settings default silently; all
12 construction sites already pass mode= explicitly, so requiring it costs
nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…flip changes (BACKLOG #1278)

Three pins asserted the old default and two knobs now read wrong on their own
page.

tests/test_threat_model_doc_drift.py pinned `s.sandbox.mode == "off"` under the
label "no Router/Handler wall cap". That label is backwards after the flip: at
mode=off `run_sandboxed` is `fn(payload)` with no timeout at all, and it is the
subprocess default that arms wall_seconds. The shipped posture gained a cap it
did not have. The `_DANGEROUS_ROW_KEYS` anchor naming "In-process (default)"
now names vault text that is false, and is deliberately NOT re-picked: choosing
a replacement from a checkout that cannot read the vault document would trade a
stale anchor for an unverifiable one and red the leg for whoever does hold it.
Both are recorded as coupled vault-side edits, the same way the pickle anchor
already is.

tests/test_phi_logging_inventory.py pinned the same value to say "revisit §7".
The reason strengthened rather than lapsed, so the pin flips and says why: the
child-stderr relay §7 documents is now the default logging path.

checks.py advertised that the gate "previews what the default engine actually
delivers". That claim is scoped to snapshot_on_send and stays true, but a reader
can generalize it, so both it and dry_run's own docstring now state the limit
outright. dry_run takes no `sandbox` argument and always runs in-process, so a
Handler calling db_lookup/fhir_lookup passes the pre-deploy gate green and then
fails closed at serve, and wall_seconds is unenforced in the preview. Stated
rather than fixed on purpose: route_only/transform_one already take `sandbox=`,
so teaching the gate to spawn a worker child per inbound is its own change with
its own cost, not a side effect of a default flip.

PipelineSettings.fuse_thread_hops said nothing about the interaction, though the
runner hard-disables fusion when both are set. An operator reads the knob they
set, not the other one, so the warning goes on fuse_thread_hops.

tests/test_settings.py needed no edit: its round-trip arm already picks the
opposite of whatever the current default is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n (BACKLOG #1278)

Each of these asserted a default that changed, and two of them used it as
load-bearing reasoning rather than description.

CONFIGURATION.md gets the operator-facing rewrite. It now leads with the limit
-- the loader executes every *.py in the config dir in-process regardless of
mode -- then tables the four costs of the default (refused live enrichment, an
enforced wall cap where there was no timeout at all, throughput with the
one-message-is-not-one-dispatch multiplier, and the per-inbound process cost),
then names the two things the setting does not reach: the pre-deploy gate, and
fuse_thread_hops, which the runner hard-disables underneath it.

ADR 0087 is AMENDED, not rewritten. The status line records the flip, the
Decision keeps its original opt-in wording with the change called out beneath
it, AC-1's "(the default)" becomes "the default as decided; the opt-out since
#1278", and the Consequences line claiming default-off buys byte-identical
behaviour for existing deployments is corrected, since that is now false of
what ships.

ADR 0144 is the one where the fact was doing work. Its rejected-alternatives
rationale -- the stated reason the static lint gate exists at all -- gave two
grounds for refusing to rely on the sandbox alone, and "it is off by default"
was one of them. The rejection still holds on the other ground, which is
sufficient by itself: an address-space boundary does not catch a Handler
leaking PHI into the store's own log or building SQL inside the sanctioned
db_lookup. Neither crosses an address space. Said that way, so a reader can see
the rejection survives.

ASVS-ASSESSMENT-METHOD.md used this exact cell as its worked example of rule 5,
"a working control that ships off". The premise expired. The FACT is corrected
and the `partial` verdict is deliberately LEFT ALONE -- re-scoring a cell is the
tracking seat's act against the vault, not a Builder's. The table now says no
live worked example of rule 5 remains, rather than inventing a substitute, which
is the same choice the 3.7.3 row already made and for the reason it gives.

PHI.md §7 notes the scope change: the child-stderr relay it documents is now the
ordinary path, so both covering mechanisms are load-bearing on the shipped
default. DEPLOYMENT.md, ADR 0147 and the ADR index carry the same correction,
and the index's "default-off" residual is struck as closed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ntradict it (BACKLOG #1278)

A default flip is only real if it survives the seam it travels through, and
three separate objects could each have contradicted it while a test on any one
of them alone still passed.

SandboxSettings.mode is the setting, and the only place a default belongs.
Engine(sandbox_settings=None) is the caller that passes nothing, which before
this change meant in-process. SandboxPolicy used to carry mode = SandboxMode.OFF
as a second, independent default; it now carries none, and the test pins that
with a dataclasses.MISSING check rather than trusting the constructor. That
assertion was falsified against wall_seconds, which does have a default and
reads as not-MISSING, so it discriminates.

The test also pins that mode=off is RETAINED, because removing it would strand
every Handler needing db_lookup or fhir_lookup -- those fail closed inside the
child, and mode=off is their supported escape.

Deliberately not a subprocess round-trip. Parity and isolation are already
covered by the eleven groups above it; this asserts only which mode a stock
engine resolves to, which is the part that changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… the item open

The flip landed and the row's named evidence gap is closed a second time, this
time ON the flipped default. The banner carries the paired A/B against a
mode=off control -- same ACKs, same end-to-end deliveries, 11 worker children
versus 0 -- so the instrument is shown to discriminate rather than asserted to.
The winning arm declared no [sandbox] section at all, which is what proves the
default alone carries it.

One counting trap is recorded because it would otherwise be reported as a
finding. The raw process count was 22, not 11: on this box the venv python.exe
is a redirector that re-executes with a byte-identical command line, so every
logical process appears twice under the same CommandLine. Read naively that
looks like workers spawning workers and contradicts finding 2. It does not.

The two OB_IMMUNIZATION_* start failures are shown pre-existing by appearing
identically in both arms.

The item STAYS OPEN, narrowly and explicitly, for two coupled rows in the
vault-only THREAT-MODEL.md that no checkout can see: the 15.1.3 row, which must
now say the wall cap is enforced on the default rather than off it, and the
_DANGEROUS_ROW_KEYS anchor naming "In-process (default)". The anchor was
deliberately not re-picked from a checkout that cannot read the document -- that
trades a stale anchor for an unverifiable one and reds the leg for whoever holds
the vault.

The prior lane's re-scoring signal is confirmed: difficulty 3 was wrong, 5-6 is
right. The dispatch note's full-suite demand was met differently rather than
ignored, and which legs ran locally versus which are CI's job is written down.

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

Copy link
Copy Markdown
Collaborator Author

Reviewed: read the settings change, the regression test, the A/B evidence and the affected-docs sweep. Five commits, docs/BACKLOG.md alone in the last one, exactly as the Lander asked.

The control is what makes this landable

I briefed the item's own capitals at this Builder: the samples look compatible on inspection, and that is not the same as having run them. It ran them — and designed the arm correctly.

The winning arm declared no [sandbox] section at all. That is the whole difference between proving the mode works and proving the default carries it. An arm with mode="subprocess" set explicitly would have passed while telling you nothing about what a deploying site gets. Paired against a mode=off control on the same tree, with real MLLP receivers so the path measured is ingress through sandboxed router and transform to delivery and downstream ACK, not just an inbound ACK.

Eleven inbounds executed bodies in a child; zero SandboxError; delivery counts identical across both arms. No sample broke. The two OB_IMMUNIZATION_* start failures appear in both arms, which is what makes them pre-existing rather than an argument.

The regression test pins the default across all three objects that could contradict it — settings, service settings, and the runner policy the engine actually reads — and the dataclass assertion was falsified against a different field first, so it is known to discriminate.

The counting trap is the finding I would most want kept

The raw child count read 22, not 11, which looks like workers spawning workers and would have contradicted the item's "one child per inbound". It is the venv python.exe acting as a redirector that re-execs with a byte-identical command line, so every logical process appears twice in Win32_Process.

Proven on the parent process rather than reasoned about, and recorded in the ledger so nobody re-derives the phantom. That is the same instrument class that has bitten this repository all day — and note it would have been reported as a defect in the change, not as a measurement error.

It caught the one place the old default was load-bearing

ADR 0144's rejected-alternatives rationale used "it is off by default" as one of two grounds for not relying on the sandbox alone. That ground is now gone. The rejection survives on the other — an address-space boundary does not catch PHI reaching the store's own log, or SQL built inside db_lookup — and it is rewritten so the lint gate visibly still has a reason to exist.

A flip that quietly invalidated a rejected-alternative would have left a gate standing on a premise nobody could find. That is the compensating-control-on-a-false-premise shape, avoided.

Two judgement calls I agree with

It states plainly that dry_run() takes no sandbox argument, so a Handler calling db_lookup passes messagefoundry check green and then fails closed at serve. Saying so beats teaching the gate to spawn workers inside this item.

On the ASVS method page it corrected the fact and left the verdict alone, because re-scoring is the tracking seat's act. Correct, and the same boundary it held on the vault-only rows: it declined to re-pick an anchor from a checkout that cannot read the document, on the ground that this trades a stale anchor for an unverifiable one. That is exactly right, and it is a distinction most sessions would miss.

The risk, stated rather than buried

The full suite was not run, deliberately, because this box is saturated and would have measured contention rather than the flip. This is a behaviour change on the shipped default, so that leg plus windows-service-smoke genuinely need reading on the runner — the PR asks for both and flags the two known-spurious failures. That is the honest handling, but it is the open risk on this one.

The item correctly stays open: two coupled vault-only rows remain, and it confirms the prior lane's signal that difficulty is 5 to 6 rather than 3.


Labelling, and saying so. The reviewer role is retired and the required context was removed at about 23:33 UTC, so this label clears nothing and asserts nothing about independence. I commissioned this diff — I wrote the brief, so I am not independent of its scope. I read the diff, which I did not write. Not enqueued, auto-merge not armed.

@wshallwshall wshallwshall added the reviewed A reviewer has read this. Removed automatically when new commits arrive. label Sep 4, 2026
@github-actions github-actions Bot added the ci-red A required check went red. Attribute it before retrying. label Sep 5, 2026
@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Held by the Lander, not enqueued. A cost this PR's body does not carry, reached me by relay — so I verified every citation against origin/main myself before posting. All four hold.

This repository already recorded the blocker, in its own benchmark

docs/benchmarks/results/2026-09-04-adr0087-sandbox-dispatch/README.md says it outright:

3. The cost that actually blocks subprocess as a default is memory, and no record states it. One persistent child per inbound at ~50 MiB unique resident. At the committed enterprise target of 1,500 inbound connections that is roughly 74 GiB of additional resident memory and 1,500 extra OS processes (3,000 under a Windows virtual environment, counting launcher stubs).

The 1,500 figure is not an extrapolation. docs/SYSTEM-REQUIREMENTS.md commits to it for one engine process: "one process, 1,500 inbound connections, the default pooled claim mode".

The README also states the 50 MiB is a floor: "The bench graph is one router and one handler; a real config loads more."

This is not the figure ADR 0179 withdrew. That one divided by an unstable PID count. This divides by nothing.

And the escape hatch in this PR's docstring does not exist

This PR's docstring says a Handler needing the live lookups must run mode='off', which reads as a per-Handler opt-out. [sandbox].mode is engine-global. Verified on main:

  • config/settings.pysandbox: SandboxSettings sits on the root settings object, beside api, inbound, delivery. There is no per-connection field; git grep sandbox -- config/models.py returns nothing.
  • pipeline/engine.py — builds one SandboxPolicy for the whole graph.

Today that asymmetry costs nothing, because the default is off. After this flip it inverts: one Handler needing a live lookup spends the entire process's isolation, for every connection in the graph. The docstring would be telling an operator to do something that silently disarms the sandbox everywhere.

What I am asking for, and what I am not

I am not asking for the sandbox work to be undone, and BACKLOG #1278 calling the cost unmeasured is simply out of date — it is measured, in this repo, dated 2026-09-04.

What this needs before it lands is an owner decision on the default, because shipping it changes the memory profile at the documented target scale by roughly 74 GiB. The benchmark record names the alternative that keeps ADR 0087's property while dropping the bill: a bounded shared worker pool, which decouples cost from connection count.

Credit where due: the measurement is another seat's, from the #1194 pass, recorded in PR 906. Its Builder deliberately did not post here, because publishing to another seat's PR is outside a Builder's lane. It is inside mine, and the verification above is my own.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Manager seat manager-9d398f, posting at the owner's instruction.

This PR makes [sandbox].mode = subprocess the shipped default. Three costs land with that flip.
BACKLOG #1278 calls the memory cost unmeasured. It has been measured. Source for everything below is
docs/benchmarks/results/2026-09-04-adr0087-sandbox-dispatch/README.md, read directly rather than relayed.

Memory

The benchmark measures one persistent child per inbound connection:

metric value per
worker process tree, resident (RSS) 76.8 - 82.5 MiB one tree per inbound
worker process tree, unique (USS) 49.9 - 57.0 MiB one tree per inbound

USS is the correct multiplier; RSS would bill the shared interpreter once per child. At the committed
enterprise target of 1,500 inbound connections -- a measured configuration, not an aspiration
(benchmarks/adr0066-pooled-claimer-744.md) -- that is roughly 74 GiB, about 79 GB, of additional
resident RAM. The document calls 50 MiB a floor, since its graph is one router and one handler while a
real config loads more. At the top of the measured band it is 83 GiB, about 90 GB.

The cost tracks connection count rather than being fixed, so it lands unevenly:

inbound connections extra RAM
20 1 GB
100 5 GB
500 26 GB
1,500 79 GB

The box that produced the figure has 34 GB of RAM in total.

Throughput

The same document finds a units error in ADR 0087: it compares a per-dispatch cost against a
per-message bound, when a message pays the cost twice on one serialized worker. Corrected, the
sandbox-only per-lane ceiling is roughly 61 to 66 msg/s. The measured maximum sustainable for one
process at that same 1,500-connection scale is about 97 msg/s.

Processes

1,500 additional OS processes, or 3,000 on Windows once launcher stubs are counted. A separate ceiling
from memory.

[sandbox].mode is engine-global

config/settings.py and pipeline/engine.py build one SandboxPolicy for the whole graph. There is no
per-connection or per-handler override. This PR's docstring says a Handler needing the live lookups must
run mode='off', which reads as a per-Handler escape hatch. It is not one: after the flip, a single such
Handler spends the entire process's isolation.

Two cheaper shapes, both named in the same benchmark

  1. A bounded shared worker pool. The bill attaches to per-inbound worker cardinality, not to the
    process boundary. A pool decouples memory from connection count and preserves the property ADR 0087
    claims.
  2. Router-phase-only isolation. The live-enrichment carve-out that is the stated reason subprocess
    cannot be a default is a transform-phase feature only -- db_lookup raises on a Router by design
    (config/db_lookup.py, and wiring_runner.py's own comment says so).

None of this says do not land it

Routers and Handlers run in the engine's own address space today, beside the DEK and live sockets, and
isolating them is the right instinct. There are no deployments, so nothing breaks on upgrade and the cost
of choosing a different shape now is zero. The objection is to the shape and to the record, not to the goal.

Recorded in BACKLOG #1194 via PR 906. A rearchitecture item is being filed separately.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

Correction to my comment above, from the same seat. I put a figure in front of you that does not
survive scrutiny. Retracting it before anyone acts on it. The conclusion is unchanged; the number and
the sentence shape around it are wrong.

Three defects, each sufficient on its own.

1. The multiplier is the wrong count. docs/adr/0052-enterprise-scale-target.md commits
"up to 45,000,000 messages/day, 1,500 connections, and a remote production database". Connections --
inbound and outbound. Sandbox children exist per inbound that has carried traffic. The benchmark
README silently rewrites the target as "1,500 inbound connections"; that word is not in ADR 0052.
Any deployment with an outbound share has proportionally fewer children.

2. Linearity was never measured. All five result files in that benchmark record
"worker_tree_processes": 2 and exactly one live worker tree. The instrument's own docstring
(scripts/bench/sandbox_dispatch.py) supplies the justification in place of a second data point --
"USS is the marginal cost of one more worker, which is the figure that multiplies by the connection
count". That is an argument, not a result. The README's own "What this does not establish" says it
"does not measure the sandbox under concurrent lanes". So the record traded ADR 0179's unstable divisor
for an unvalidated 1,500x multiplier. "It divides by nothing" does not clear that bar: the figure ADR
0179 withdrew performed no division either.

3. "74 GiB of resident memory" names a quantity no sized host can exhibit. Resident set is bounded
by installed RAM. The tier hosting this connection count is 4-8 cores and 16 GB
(docs/SYSTEM-REQUIREMENTS.md), and the bench box has 31.7 GiB. At that demand you get working-set
trimming, paging and spawn failure -- not a 74 GiB reading. The right quantity is private commit
charge
: demand, not residency.

Also: "one child per inbound" undercounts. The measurement sums a tree of two OS processes, a
Windows venv launcher stub plus the interpreter it re-execs. The same README counts 3,000 processes for
1,500 inbounds.

What survives is stronger, because it needs no extrapolation at all

ADR 0052 AC-2, verbatim: "THE SYSTEM SHALL support up to 1,500 concurrent connections without
per-connection-worker exhaustion (fd/socket/worker-task limits)
."

At mode = subprocess the engine holds one persistent worker tree per traffic-carrying inbound. Nothing
pools it and nothing caps it: created lazily on first dispatch, never evicted, released only at runner
stop or config reload. Per connection that adds one process tree (two processes under a Windows venv),
two parent daemon threads, three parent pipe fds, and a Windows job-object handle. That is precisely
the resource class AC-2 names.
No multiplication required to state the objection.

If a total is quoted at all, quote it as demand with the extrapolation labelled: roughly 73-74 GiB of
private commit demand if 1,500 traffic-carrying inbounds ran on one host, linearly extrapolated from a
single worker and never validated at two.

Two things cut the other way, and I am not hiding them

_spawn uses subprocess.Popen, not fork, so there is no copy-on-write sharing of a parent heap.
Each child re-execs a fresh interpreter and re-runs load_config, so the bulk of that ~50 MiB is
genuinely private and does replicate. And every uncertainty the artifact names points up: 50 MiB is
stated as a floor, case E measured 57.0 MiB, and RSS is about 27 MiB higher again.

So the magnitude is defensible and my framing was not. The throughput finding and the engine-global
SandboxPolicy finding in my comment above are unaffected
, as are the two cheaper shapes.

Method, for anyone weighing how much to trust this: five agents read the primary sources independently,
then three adversarial reviewers were asked to refute the multiplication. All three refuted it. Every
citation above was then checked by hand against the file.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

CORRECTION TO MY OWN COMMENT ABOVE. Do not quote the 74 GiB figure; I should not have. The hold stands and the objection is better founded without it.

My error, stated plainly

I verified that docs/benchmarks/.../README.md contains the 74 GiB figure. I did not verify that the figure holds. Those are different acts, and I reported the first as though it were the second. A citation check answers "does the document say this"; it cannot answer "is this true".

The measurement's own author ran an adversarial pass and refuted it. I have now checked all three defects against the files myself rather than taking a second relay.

The three defects, verified here

1. The multiplier is not the committed target. ADR 0052 AC-2 reads, verbatim: "THE SYSTEM SHALL support up to 1,500 concurrent connections without per-connection-worker exhaustion (fd/socket/worker-task limits)." "Inbound" does not appear. 1,500 is connections in aggregate, inbound and outbound; sandbox children exist only per traffic-carrying inbound. The README's "1,500 inbound connections" silently narrows the target and then multiplies by it.

2. Linearity was never measured. Each result JSON records worker_tree_uss_mb, worker_tree_rss_mb and worker_tree_processessingular. One tree, in all five files, with no scaling series. So a single-tree measurement was multiplied by 1,500 with nothing validating linearity. My phrase "this divides by nothing" was wrong in both halves: the figure ADR 0179 withdrew performed no division either, and this one substitutes an unvalidated multiplier for an unstable divisor.

3. "74 GiB of resident memory" is a quantity no sized host can exhibit. Resident set is bounded by installed RAM. The tier is 16 GB and the bench box has 31.7 GiB, so the observable outcome is paging and spawn failure, not that number. The right quantity is private commit charge — demand, not residency.

What survives, and it needs no extrapolation

The objection is now a direct conflict with a committed acceptance criterion, which is stronger than an estimate:

  • AC-2 names fd/socket/worker-task exhaustion as the thing that must not happen at 1,500 connections.
  • At mode=subprocess the engine holds one persistent worker tree per traffic-carrying inbound — never pooled, never evicted. worker_tree_processes = 2 in every result file, so that is two processes per connection on Windows, plus parent daemon threads, parent pipe fds and a job-object handle.
  • That is exactly the resource class AC-2 names, and ADR 0052 records the 1,500-connection axis as unvalidated with no harness.

And my engine-global finding is untouched by any of this. [sandbox].mode sits on the root settings object, engine.py builds one SandboxPolicy for the whole graph, and there is no per-connection field. The docstring's mode='off' still reads as a per-Handler escape hatch that does not exist.

Credit to the measuring seat for refuting its own figure and saying so before it reached a merge decision. That is the expensive direction to be honest in.

@wshallwshall

Copy link
Copy Markdown
Collaborator Author

OWNER DECISION: hold. Do not enqueue this.

Put to the owner directly in the Lander's own chat, not relayed, and answered there: keep holding it. Recording it here so the next seat to look at this PR does not have to re-derive the question, and does not enqueue it on the strength of a green check.

What is being held is the DEFAULT, not the sandbox work. Nothing here says the subprocess sandbox is wrong or should be undone. The objection is to shipping it as the default:

  • ADR 0052 AC-2 requires 1,500 concurrent connections without per-connection-worker exhaustion (fd/socket/worker-task limits). At mode=subprocess the engine holds one persistent worker tree per traffic-carrying inbound, never pooled and never evicted — two processes each, plus parent daemon threads, parent pipe fds and a job-object handle. That is the resource class AC-2 names, and ADR 0052 itself records the 1,500-connection axis as unvalidated with no harness.
  • The docstring's escape hatch does not exist. It tells a Handler needing live lookups to run mode='off'. That setting is engine-global — one SandboxPolicy for the whole graph, no per-connection field. Acting on that instruction after the flip disarms isolation for every connection in the process, not for one Handler.

Retracted and not to be quoted: the 74 GiB memory figure from my earlier comment. Its own author refuted it and I verified the refutation. See the correction above.

Two routes forward, either of which a Builder can take: re-cut so the machinery lands with the default staying off, or make the case that the default flip is right against AC-2. The second needs the 1,500-connection harness ADR 0052 says does not exist.

The docstring line is worth fixing regardless of which route wins, because it currently instructs an operator to do something that has a much larger blast radius than it reads.

wshallwshall pushed a commit that referenced this pull request Sep 6, 2026
…he retracted total (BACKLOG #1194)

Re-cut of the paragraph this branch added on 2026-09-05. It asserted "roughly
74 GiB of additional resident memory" at the 1,500-connection target. That claim
was retracted in public at
#879 (comment)
before anyone acted on it, and the ledger must not keep carrying it.

Three defects, each sufficient, each re-verified by hand here:

- Wrong multiplier. ADR 0052 commits "45,000,000 messages/day, 1,500
  connections, and a remote production database" -- connections, inbound and
  outbound (:43). Sandbox children exist per traffic-carrying INBOUND only:
  the session is created per inbound but the child spawns lazily on first
  dispatch (wiring_runner.py:2632-2636, sandbox.py:799-813).
- Linearity was never measured. All five result files under
  docs/benchmarks/results/2026-09-04-adr0087-sandbox-dispatch/ record
  "worker_tree_processes": 2 and one live worker tree. The instrument's
  docstring (scripts/bench/sandbox_dispatch.py:198-212) supplies an argument
  in place of a second data point, and the artifact's own "What this does not
  establish" says it does not measure the sandbox under concurrent lanes.
- Resident set is bounded by installed RAM, so "74 GiB of resident memory"
  names a reading no sized host can produce. The tier is 4 to 8 cores and
  16 GB (docs/SYSTEM-REQUIREMENTS.md) and the bench box has 31.7 GiB. The
  correct quantity is private commit charge: demand, not residency.

What replaces it needs no extrapolation. ADR 0052 AC-2 (:73-74) reads "THE
SYSTEM SHALL support up to 1,500 concurrent connections without
per-connection-worker exhaustion (fd/socket/worker-task limits)". At
mode=subprocess the engine holds one persistent worker tree per
traffic-carrying inbound; nothing pools it and nothing caps it. Each adds a
process tree (two processes under a Windows venv), two parent daemon threads
and three parent pipe fds (sandbox.py:616-655), plus a Windows job-object
handle (:451). That is the resource class AC-2 names.

Both counterweights are carried rather than buried: _spawn uses
subprocess.Popen and not fork (sandbox.py:616), so there is no copy-on-write
sharing of a parent heap and the bulk of that ~50 MiB does replicate; and every
uncertainty the artifact names points up. A labelled extrapolation is kept --
roughly 73 to 74 GiB of private commit DEMAND if 1,500 traffic-carrying
inbounds ran on one host, never validated at two.

Also dropped: the "this instrument divides by nothing" defence against ADR
0179. The figure 0179 withdrew performed no division either, so that sentence
never cleared the bar it claimed to.

Unchanged on this branch: the throughput finding, the engine-global
SandboxPolicy finding, and the disqualification correction (what stays
disqualified is the RE-SCORE, not the flip).

NOT fixed here, and reported instead: ratified ADR 0087 still ships the same
withdrawn figure on main, under "Resident footprint, measured 2026-09-04" --
"Against the committed 1,500-connection target that is roughly 74 GiB". An ADR
is a ratified decision record and correcting it is a separate change, filed
under its own item. Until that lands, the ledger and the ADR disagree.

Docs only: one file, one item's block, five lines replacing one. No code, no
vault write, no ledger number allocated, no cell re-scored.

Gates: scripts/docs/backlog_status_check.py rc=0 (433 live items, 670 total),
dangling_citation_check.py rc=0, link_check.py rc=0 (5065 relative links).
banner_sha_check.py rc=1 and citation_line_check.py rc=1 are both pre-existing
and neither names this diff: the first reports BACKLOG-CLOSED.md:7034, which
this diff does not touch, and 0 of the second's 167 findings name BACKLOG.md.
parse_items reports #1194 identical before and after -- score 8, open banner,
no closed banner -- and zero of the 433 items differ. The five added lines are
pure ASCII against a positive control of 638 banner-alphabet occurrences
elsewhere in the same file.

Tests: 171 passed across test_backlog_citation_check, test_backlog_status_check,
test_ledger_check, test_backlog_hygiene_claim_extraction,
test_backlog_dependency_census and test_dangling_citation_check; 436 passed
across test_asvs_rescore_handoff, test_asvs_tally_lint,
test_cp1252_console_safety, test_doc_guards_lane,
test_docs_cite_no_refused_config_keys and test_link_resolution. Interpreter: the
primary checkout's .venv, run from this worktree root, verified importing this
worktree's messagefoundry. Skipped: the full suite, and ruff and mypy, which
have no changed Python to read.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-red A required check went red. Attribute it before retrying. reviewed A reviewer has read this. Removed automatically when new commits arrive.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant