fix/notes-triage-top3 #3

Merged

troed merged 13 commits from fix/notes-triage-top3 into main

2026-10-12 23:14:14 +02:00

troed commented

2026-10-12 23:13:56 +02:00

Owner

Fixes from agent feedback

troed added 13 commits

2026-10-12 23:13:56 +02:00

fix(client,mcp): remote calls use the per-method timeout, not the relay launch window 27ed81e1ad

The MCP front door passed no timeout to the remote transport, so every
remote call fell back to SshRelayTransport's own timeout - the ~90 s
launch window - while the equivalent local call got 600 s. A remote
shell or action that legitimately ran for minutes died at 90 s no
matter how large the action's own timeout was (Oct-12 mastodon note).

The per-method budgets now live in client.METHOD_TIMEOUTS (pinned
against SocketClient's signature defaults by a test, so the two cannot
drift), and the front door passes method_timeout(name) on both the
first call and the retry. The transport's own timeout still bounds
spawn + handshake.

fix(engine,mcp): egress enable/disable, and handlers read .umwelt.toml 33e5103eb9

Three Oct-12 notes (schjessuwiki, matrix) traced to one gap: no tool
op could turn egress on, and handle_egress worked from the EgressPolicy
State.init loaded, so save_override_state rewrote the whole file from
that copy - silently resetting a hand-set enabled = true to false,
dropping any rule hand-added since startup, and leaving 'list' reporting
enabled = false while the file said true (until a control-plane
restart).

umwelt_egress gains op=enable/disable (idempotent, no other arguments),
and every op now reads the workspace's .umwelt.toml as it is now:
_fresh_egress() replaces st.override_egress in the handler and _egress_list
reports the policy it is handed. Notices state the recreate boundary:
enabling joins the egress network at the next sandbox recreate, and a
disable leaves the running sandbox's network and kernel rules in place
until then.

fix(egress): direct tcp destinations join NO_PROXY a8143d3c6a

Three notes (Oct-8 conduct, Oct-11 llama, Oct-11 matrix): NO_PROXY was
loopback-only, so a client that honors the proxy env (curl, kubectl, git)
proxied even destinations the kernel already allowed directly - a MITM
403 for kubectl, and 'curl: (52) Empty reply' on a large POST to the
local llama.cpp router, costing several 130k-token benchmark runs.

egress_env now takes static_tcp - the create-time rules, already
kernel-enforced - and appends each host:port to NO_PROXY; the shell exec
path passes st.egress.static_tcp. Relayed rules (added after create) stay
proxied on purpose: dialing one straight would replace the gateway's
explicit verdict with a bare drop, which is the failure mode the
loopback-only rule was added for in the first place. proxy_env stays as
it is - the A2 plan records it as retained-but-unused API ('do not
delete').

fix(client): first read on a cold relay channel gets the transport budget 0cbde8e3cc

Review finding on the per-method timeout change: the relay answers the
handshake *before* it launches the agent and creates its container, so
the first request's reply waits out that launch (up to the ~60 s launch
window). Passing only the per-method budget made a cold first call
worse than before it - a 60 s budget for list/read_file/etc. equals the
launch window, which client.py's own comment forbids (the transport
times out at the same moment the relay would report the real reason,
and request()'s teardown then kills the agent it just launched).

SshRelayTransport.request now floors the first read (the cold one) at
its own timeout and uses the per-call budget from the second request
on; the launch window keeps its purpose and the call budget keeps its
own.

fix(engine,egress): review round on the egress fixes f678444790

Code review of the batch found four issues, all fixed here:

- The handler reported the file but left State.override_egress stale, so
  op=enable on a hand-edited enabled = true answered 'enabled: false'
  next to a notice saying 'already enabled', and the next container was
  still built with --network=none (run_args/recreate read the cache).
  _fresh_egress now refreshes the cache as it reads, every handler path
  reports the policy it just read, and the list read is lock-held.
- The enable notice promised that a recreate joins the egress network.
  It does not: only the control plane's own start creates the network
  and gateway (engine.recreate is rm + run with the current args), so an
  in-place recreate - a persisted grant, a first ingress enable - leaves
  the sandbox on --network=none. The notice, CONFIGURATION.md and the
  setup steps now name a control-plane restart.
- save_override_state wrote .umwelt.toml with a plain write_text, which
  truncates first; a concurrent egress read could see an empty file and
  silently report enabled: false (serve() threads every connection).
  Now tmp + os.replace, like write_policy_file. Reproduced by a
  writer/reader test before the fix.
- run_args mounted the egress CA bundle without the missing-source guard
  the mount loop above uses, so a stale-enabled policy on a host without
  the bundle let podman create that host path.

Also: the relayed-rule NO_PROXY test now carries an actual relayed rule
(it asserted the loopback-only case with no rule at all), the toggle
audit lines have a test, and proxy_env's docstring records that it is
the superseded rev-A env whose NO_PROXY contract egress_env replaced.

fix(engine,egress): the policy cache never runs ahead of the file 974cdcf53c

Re-review of the previous round found that adopting the file's policy
into State.override_egress and then mutating it in place made a *failed*
save leave the cache describing a policy the file never got: the save
path's strict loader refuses a hand-edited [[mounts]]/[[remote]] typo
after the rule was already appended, and a later in-place recreate
renders the kernel ruleset from that cache - so the sandbox would
enforce a rule that list (which reads the file) reports as absent.

Handlers now adopt the freshly read policy immediately and mutate a
copy_policy() of it; _egress_persist adopts the copy only once the save
has succeeded. Both directions are pinned by tests that fail with the
cache diverging (add and enable).

Also from the re-review: the atomic write keeps the target's mode (a
tightened 0600 is no longer loosened to the tmp file's 0644) and unlinks
its tmp file if the replace itself fails (it would surface as untracked
git noise); client.py's cold-path wait expression loses a redundant
local; a disable_ with no running network owes no effect clause; and
CONFIGURATION.md disambiguates the two senses of 'recreate'.

feat(engine): per-project [resources] caps 9c3e6174fb

Oct-6 starfleet note: in-sandbox ESP-IDF builds need ~6g/6cpu against a
2g/2cpu base, and the only way to raise the caps was the machine-local
overlay - which applies to every sandbox on the host. There was no way to
scope it to one project.

A project's (gitignored) .umwelt.toml may now carry a [resources] table
(memory, cpus, pids_limit); it overrides the configured caps for that
sandbox alone and follows the overlay's partial-table semantics, inherited
values keeping their type. Validation is shared with the base config
(check_resources), a bad value is an overrides problem rather than a
startup failure, and save_override_state re-emits the hand-written table
so the wholesale rewrite on a grant/egress/ingress change cannot drop it -
the clobber class the egress enabled flag just came out of. The
egress/ingress gateways keep the configured caps.

feat(egress): warn when a tcp rule points at the host's own address ed003def72

Oct-11 matrix note: a remote sandbox on server S cannot reach S's own LAN
address - rootless podman masquerades container traffic to the host's
outbound address, so the connection arrives with src == dst and S's stack
refuses it. Internet egress from the same sandbox works, so the failure
is opaque and cost real debugging time.

Gap 1 of docs/specs/2026-10-11-remote-egress-followups.md, which the
parked branch held: brought to main, with item 2 (the NO_PROXY gap, fixed
earlier in this branch) marked resolved. Hairpin NAT stays out - it needs
root, and a sandbox cannot reach it.

Detection lives in write_hooks, which already holds the resolved
(host, ip) pairs at create and recreate, so it costs no extra DNS;
list stays DNS-free by reporting the stored result. The warning reaches
both the control-plane log and umwelt_egress {op: list}'s new warnings
field, which is where an agent can act on it.

feat(remote): the server floor mounts ~/.config read-write d26afc2ad8

Oct-11 matrix note / follow-ups spec gap 3: a remote sandbox on a server
that also administers its own services could not run tools that write a
config dir - pgcli died with 'Read-only file system' on the read-only
~/.config reference mount.

The server's floor config (written by remote-install.sh) now mounts
$HOME/.config read-write and says why; the committed local base keeps its
read-only reference mount, and the test pins that asymmetry. Existing
servers pick the change up by re-provisioning (--force) or flipping the
flag by hand.

fix(engine): a recreate notice says it killed the sandbox's processes 3b458093e7

Oct-10 llama note: a long benchmark sweep died silently when a grant
triggered an in-process recreate - the notice said only 'sandbox
recreated, effective now'. A recreate replaces the container, so
everything running inside it dies with the old one; the grant/revoke
effect clause and the first-ingress-enable notice now say so, and the
CONFIGURATION.md quirk list gains the rule (a long job belongs in a
detached worker writing a durable log in the workspace, since anything
whose output lives only in the container is gone with it).

fix(engine,remote): review round on the second batch bff930f756

Code review found one Critical and several smaller items.

Critical: a bad [resources] value broke every persisted write. The save
path re-reads the override file to re-emit the tables it does not own,
and that re-read validated the table - so one typo (`cpus = -1`) turned
every grant/revoke/egress/ingress call into a mount-grant-shaped
ConfigError naming resources.cpus, while the container had already been
recreated and the file never changed. The raw reader is now exactly that
- raw and unvalidated, mirroring what the tolerant State.init path
reports as an override problem - and check_config gained the loader it
was missing, so umwelt-ctl check-config catches it instead of a grant.

Also: egress list's local-address warnings are derived from the current
policy (a removed rule no longer warns; a rule added live is not in the
create-time render, which the docs now say); local_ipv4_addresses
returns empty rather than raising if the interface lookup fails, since
nothing in the create path may fail for an address lookup; write_hooks'
parameter is named `resolved` (it is (host, ip), while the state field
it feeds is (host, port)); the .umwelt.toml digest notice and audit line
include [resources] (a caps-only edit used to report 'no mounts'); the
remote ~/.config decision is labelled as what it is - dropping the
read-only flag, an ungated rw mount of a host directory, credentials
included - and the gh/fj/'committed [resources] table' doc lines no
longer claim read-only or committed-only.

feat(remote): nest umwelt's config dir read-only; document the tier-3 quirks d5f6c5e488

Review follow-up on the read-write ~/.config decision: the server-side
agent runs as the host user, so that mount is an ungated read-write
window onto a host directory - and $HOME/.config/umwelt holds host
policy (the remote registry, egress/ingress overrides) that a sandbox has
no business rewriting. A nested read-only mount of $HOME/.config/umwelt
wins over its read-write parent (run_args skips it when the directory is
absent, like any other missing bind source), and both the spec and
CONFIGURATION.md now state the trade rather than describing it as a
'writable config dir'.

With it, six recurring findings from the agent feedback log that were
never documented: Go's container-local GOPATH and the forbidden module
proxy, uv's unwritable default cache, the blocked npm registry, recursive
grep/find walking network mounts, the missing systemctl and git
credentials, and upstream test suites whose network fetches hang in the
sandbox (llama.cpp's test-arg-parser). Each names the workaround the
reporting agent had to find for itself.

docs: the uv quirk names the socksio dependency, not just the cache 08234613ab

The note log's reflect entry was the last place that said an httpx-based
test needs socksio because the sandbox's HTTP env routes through the
proxy; with the entry retired (its uv-cache half is already in the list),
the requirement belongs in the quirk bullet itself.

troed merged commit fb211207ed into main

2026-10-12 23:14:14 +02:00

troed deleted branch fix/notes-triage-top3

2026-10-12 23:14:14 +02:00

troed referenced this pull request from a commit

2026-10-12 23:14:15 +02:00

Merge pull request 'fix/notes-triage-top3' (#3) from fix/notes-triage-top3 into main