Repository navigation
docs: concepts pages (PR 5c of REORG-PLAN) - #498
Conversation
Sessions and turns, brain and hands, session multiplexing, sandboxes and tiers, multi-user and dispatch archetypes, each separating what the code does today from what a spec only proposes. Linked from docs/index.md and the README; REORG-PLAN records PR 5c. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
ingpaolodettori-dev
left a comment
There was a problem hiding this comment.
Thanks, a clear set of pages, and the "what is built vs what is proposed" framing is the right one. I checked the claims against main (5a29af3), and most hold up with file:line evidence. Three points are wrong in ways an operator could act on (four must-fix inline comments), so I'm requesting changes:
- Silent workspace reset. brain-and-hands.md:55 and sandboxes-and-tiers.md:72 say a removed workspace is always reported (
workspace_reset). Idle reclaim is silent on both tiers (#338), as the MicroVM paragraph on the same page already says. - Private-network reach. brain-and-hands.md:66 says container sandboxes can't reach the host's private networks. That holds only on Kubernetes. On the VM and Compose, forwarded traffic is unfiltered.
- Bundle ownership. multi-user.md:55 says "there is no ownership record". There is one (the budget entry), and it drives the 403 on delete mentioned six lines earlier.
Plus the smaller corrections inline: the 15 s drain falls inside the 20 s rather than after it, leastInFlight reads no head, the P5 spec calls cwd inert, and the unknown-session table splits on auth, not path.
Your "Please check" items:
X-SH-Subjectfor unauthenticated callers: confirmed absent. The only reader is thesubject_conflictcheck (turn-auth.ts:249-252); the no-token path returns null. Keeping the page as written is accurate.directmode underMOCA_TENANCY=multi: not refused. The mode depends only on whether an injector is configured (control-plane/src/exchange.ts:232-235). Since multi-user.md says the worker holds the real key in direct mode, a sentence noting that multi-tenancy doesn't force placeholder mode would help.- ADR-0035/0040 Proposed: no change requested here.
Note: main gained #494 (workload runs on shared sandboxes) after this branch's base. Nothing on these pages is made wrong by it; the API-token row for /workloads still holds (a workload run uses the session token).
Author: pdettori (MEMBER, maintainer) · Docs only · No .claude//.vscode/ changes · 1 commit, DCO signed · CI green
| ([Configuration](../reference/configuration.md)). The per-session directory is **not a security | ||
| boundary**: sessions on one sandbox share a Unix user and container. An idle workspace is deleted after | ||
| `SH_WORKSPACE_IDLE` (30 minutes by default; the microVM installer sets 8 hours). A session whose | ||
| workspace was removed or moved gets a fresh one, and the turn reports `workspace_reset`. |
There was a problem hiding this comment.
must-fix: "removed or moved … the turn reports workspace_reset" holds only for an affinity move. workspace_reset comes only from selected.workspaceReset, which is set on the detached/retiered fallback (harness/src/select-sandbox.ts, run-turn.ts:260-271). Idle reclaim removes a workspace silently on both tiers: remote-worker/internal/exec/workspace.go ("Removal is idle-only (#338)", no signal to the harness) and deploy/microvm/P4-ON-P6.md:411-426 ("a later turn is not told it happened … The silent reset remains"). This also contradicts the MicroVM limits paragraph in sandboxes-and-tiers.md. Suggest: "An idle workspace is deleted after SH_WORKSPACE_IDLE … and the next turn is not told (#338). A session moved to another sandbox gets a fresh workspace and the turn reports workspace_reset."
| environments. The model credential never reaches a sandbox. | ||
| - There is **no credentialed-egress proxy** for P6 sandboxes yet. Network reach depends on the tier | ||
| (see [Sandboxes and tiers](sandboxes-and-tiers.md)): container sandboxes can generally reach the | ||
| internet but not the host's private networks, and microVM guests have no network at all. |
There was a problem hiding this comment.
must-fix: "container sandboxes can generally reach the internet but not the host's private networks" is true only on Kubernetes (deploy/k8s/base/sandbox.yaml:77-79 excludes 10/8, 172.16/12, 192.168/16, 100.64/10, 169.254/16). On the single VM the nftables table only drops traffic to the host itself, and forwarded traffic is "untouched in S1" (deploy/vm/setup-vm.sh:713-744). So LAN/private ranges and link-local (cloud metadata) stay reachable. Compose has no private-range filter either (docker-compose.yml:205-207). Your own per-platform list in sandboxes-and-tiers.md is right; this summary overstates the isolation, which matters for anyone deciding whether to put a sandbox on a network with internal services. Suggest: "on Kubernetes container sandboxes reach the internet but not private ranges; on the VM and Compose they can reach anything the host can route to, except the host itself (VM)".
| with reason `detached`. If the sandbox has been relabelled to another declared tier, the session falls | ||
| back immediately with reason `retiered`. | ||
|
|
||
| A session never silently loses its workspace: it either waits, or is told it was reset. |
There was a problem hiding this comment.
must-fix: "A session never silently loses its workspace" is contradicted by this page's own MicroVM limits paragraph (#338) and, for the container tier, by remote-worker/internal/exec/workspace.go (idle sweep, no signal). It holds only for affinity moves. Suggest: "An affinity move never loses a workspace silently: the session either waits or is told it was reset. Idle reclaim (SH_WORKSPACE_IDLE) is still silent on both tiers (#338)."
|
|
||
| Config bundles are content-addressed and counted against a budget (16 MiB per user, 64 MiB in total by | ||
| default, 8 MiB per bundle), expiring after 30 days unless used. A bundle's digest acts as a bearer | ||
| capability; there is no ownership record. |
There was a problem hiding this comment.
must-fix: "there is no ownership record" is wrong, and it contradicts line 49 (403 for deleting someone else's bundle). The budget entry stores <bytes>:<subjectHash of first uploader> (packages/control-plane/src/bundle-budget.ts:29, read back by bundleOwnerHash :57). deleteConfigBundle returns 403 unless the caller is that uploader or an admin (handlers.ts:707-717). A bundle stored by /promote has no budget entry, so only an admin can delete it. If the point is that using a bundle needs only its digest, say that: "Using a bundle requires only its digest (a bearer capability). Deleting it requires being its uploader or an admin."
| - **The supervisor dies:** workers exit when they lose their link to it, and the process manager restarts | ||
| the set. Re-adopting running workers is deliberately not built. | ||
| - **A graceful stop:** the supervisor drains workers and waits up to 20 seconds for idle; a detachable turn | ||
| gets up to 15 more seconds before it is aborted with "harness restarting". |
There was a problem hiding this comment.
suggestion: the 15 s is not "more" than the 20 s. DETACHED_DRAIN_MS = 15_000 starts when the worker gets drain and "Must stay below the supervisor's SHUTDOWN_GRACE_MS (20 s)" so the abort lands inside the grace (packages/knative-server/src/worker.ts:108-115). Suggest: "…waits up to 20 seconds for idle; within that, a detachable turn is aborted with "harness restarting" after 15 seconds."
|
|
||
| `async: true` and the cron on-ramp need the Redis-Streams drainer that ran as a KEDA `ScaledJob` and the | ||
| `CronJob`; neither has a P6 counterpart yet, and the supervisor only runs `--role=turn` workers (a | ||
| `--role=leaf` role is reserved and not implemented). An authenticated `async: true` request returns 501 |
There was a problem hiding this comment.
nit: an anonymous async: true on a deployment with SH_REQUIRE_AUTH off still gets 202 and is queued (server.ts:887-899; the 501 needs a token or require-auth, :1106-1119). On P6 nothing drains it, so /runs/status answers queued forever. Worth one sentence so nobody mistakes the 202 for support.
| Be clear about what is and is not enforced today: | ||
|
|
||
| - Only `LANG`, `LC_ALL`, `LC_CTYPE` and `TZ` cross from the harness into a sandbox command, and the | ||
| sandbox worker removes its own settings (`SANDBOX_TOKEN`, `RELAY_*`, `SH_*`, `MOCA_*`) from child |
There was a problem hiding this comment.
nit: the worker drops an explicit list, not a RELAY_* prefix: RELAY_ADDR, RELAY_TLS, RELAY_CA_FILE, plus SANDBOX_ID, SANDBOX_IMAGE, SANDBOX_TRUST, WORKER_MAX_CONCURRENT and the prefixes SANDBOX_TOKEN_, SH_, MOCA_ (remote-worker/internal/exec/sandboxenv.go). Something like "its own settings (tokens, relay address, SH_*, MOCA_*)" would stay accurate.
|
|
||
| ## Limits on what the hands return | ||
|
|
||
| Command output is capped at 8 MiB. A command that exceeds it is aborted with exit status 137 and the |
There was a problem hiding this comment.
nit: exit 137 is for bash only (packages/k8s-sandbox/src/operations.ts:116). Other tools return the truncated output with the [output truncated] marker, or fail (read).
| | Typical host | Any Docker, podman or Kubernetes host | A Linux host with `/dev/kvm` (verified on an AWS `m8i.xlarge` with nested virtualization) | | ||
| | Workspace | Container-local; lost if the container restarts | Persists between turns in a per-workspace disk image; a fresh VM per tool call | | ||
| | Network | See below | **None**: the guest has no NIC, so `curl` and `git` to the internet fail | | ||
| | Where it runs | VM, Compose, Kubernetes | On a P6 VM host, or on an external KVM host attached to an in-cluster relay over TLS | |
There was a problem hiding this comment.
nit: the external-KVM-host-to-in-cluster-relay arrangement is documented as OpenShift only (deploy/k8s/README.md:764-774). Worth saying here.
| ## Credentials | ||
|
|
||
| A user stores an inference credential with `mocactl credentials add`. The store is **write-only**: there is | ||
| no read-back, `GET /v1/credentials` returns names, kinds, hosts and endpoints only, and the secret is read |
There was a problem hiding this comment.
nit: "only" is slightly off: the list also returns consumer and binding (handlers.ts:639-655). No secret is returned, so the write-only point stands.
- Idle workspace reclaim is silent on both tiers (rossoctl#338); only an affinity move reports workspace_reset. - Private-network reach: restricted on Kubernetes only; the VM and Compose forward unfiltered. - Bundle ownership: the budget entry records the first uploader and drives the 403 on delete; using a bundle needs only its digest. - Multiplexing: 15 s detached drain falls inside the 20 s grace, the supervisor reads no request bytes on the default policy, the 429 is its own write, drop the cwd example for the output-guard exit hazard, and S = 4 comes from the Kubernetes manifest and the Compose installer. - Unknown-session behaviour splits on authentication, not path; Redis durability on reboot; detachable turn prerequisites; 503 meanings; an anonymous async run is accepted but never drained; direct mode is not forced to placeholder under multi tenancy; smaller corrections. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
|
Addressed in 9ab41c5. Must-fix: idle reclaim is silent on both tiers (#338) and only an affinity move reports |
ingpaolodettori-dev
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround. I re-checked 9ab41c5 against the previous review (2131ec9) and main (5a29af3). It's a fast-forward on top of the reviewed commit, docs only (6 files, +58/−45), signed off, and CI is green.
All 4 must-fixes are resolved:
- Silent idle reclaim: brain-and-hands.md and sandboxes-and-tiers.md now say idle reclaim is silent (#338) and that only an affinity move reports
workspace_reset. The "What survives what" row agrees. - Private-network reach: the summary now limits the private-range restriction to Kubernetes and says the VM and Compose can reach anything the host can route to.
- Bundle ownership: the budget entry is described as recording the first uploader, and it's what drives the 403 on delete. The
/promoteadmin-only case is covered.
All 13 suggestions and nits are addressed as proposed. I spot-checked the new wording: the supervisor's only self-written response is the 429 (packages/supervisor/src/admission.ts:57, with main.ts destroying sockets otherwise). redis_unavailable 503s exist on the detachable paths (server.ts:557,705,772). The P5 spec's "fleet-wide outage" wording is quoted correctly (:348-349). The credentials list fields match handlers.ts:639-655.
One remaining nit is inline. It's on the example I suggested last round, so it's my wording to fix. Approving.
| conflicting `X-SH-Subject` is rejected), the process holds no ambient provider credential under | ||
| `MOCA_TENANCY=multi` (it is scrubbed and replaced by a sentinel), and the worker's Pi loader is locked | ||
| down. Some process-wide state in Pi cannot be isolated and is pinned by tests rather than refactored; the | ||
| [P5 design](../specs/2026-09-06-p5-session-isolation-design.md) lists the residual hazards (for example, a write error in the output guard can exit the whole process, |
There was a problem hiding this comment.
nit: I suggested this example, but the present tense overstates it. The spec says the process.exit(1) path is pinned unreachable in server mode ("Pinning the module unreachable covers both", docs/specs/2026-09-06-p5-session-isolation-design.md:349-351). So it's a hazard the tests guard against, not one that is live today. Maybe: "…(for example, a write error in the output guard would exit the whole process, which the spec calls a fleet-wide outage triggered by one session, so the module is pinned unreachable in server mode)."
Summary
PR 5c of
docs/REORG-PLAN.md: theconcepts/section. Docs only. Based onmainafter #497.docs/concepts/index.mdand six pages: sessions and turns, brain and hands, session multiplexing, sandboxes and tiers, multi-user, dispatch archetypes.docs/index.mdand the README; the plan file records 5c.Worth a careful read
The pages state several things that are uncomfortable but true of the code, with the evidence in the page:
/health, re-attach and cancel.moca-egressand per-user sandboxes are not in the code, so mutually distrusting users need separate deployments or sandbox sets.docs/migrating-from-knative.mdsays "E9 measured what the deployment tier was costing" without citing figures; I did not repeat that claim, and the guide could cite or drop it.Please check
X-SH-Subjectflow for unauthenticated callers does not appear inserver.ts; the multiplexing page says so rather than describing it as built. Is that a spec to update or code still owed?direct-mode credential exchange is refused underMOCA_TENANCY=multi(I could not verify it; the multi-user page does not claim it).Verification
scripts/check-doc-links.pyreports 0 broken links over 172 files; Prettier run on changed files. The facts come from a code and spec read (file and line citations kept in my notes), not from running a deployment.make lintnot run locally; CI will.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com