trunk-merge/pr-1164/ddebf2d4-98fe-48e8-90fd-d84e204a8193 - #1182
Closed
trunk-io[bot] wants to merge 21 commits into
Closed
trunk-merge/pr-1164/ddebf2d4-98fe-48e8-90fd-d84e204a8193#1182trunk-io[bot] wants to merge 21 commits into
trunk-io[bot] wants to merge 21 commits into
Conversation
…lity (RIG-3336) Decides whether the Compass Runner is containerized for Kubernetes, exactly what the pod is granted, and the generic object contract any conformant cluster can run. Containerize as the DaemonSet delivery + lifecycle unit, rootless with a scoped /dev/kvm device, never `privileged: true`. The ruling is explicitly contingent on a real-hardware spike (R7): a rootless HOST user has affordances a pod does not (setuid newuidmap, unrestricted unshare, no seccomp filter), so "the Runner already runs rootless" does not establish that a locked-down pod can boot the composition. If it cannot, the container-vs-host tradeoff reopens rather than a capability being granted. Three findings worth the reader's attention: - Guest RAM is POD RAM. cloud-hypervisor's virtio-fs path requires `--memory shared=on`, so guest memory is charged to the pod's memory cgroup. With low requests the pod is Burstable with a large overage, which the kubelet ranks first for node-pressure eviction -- and evicting it kills every session on the node. - Container restart implies full session teardown, forced rather than elected: without shareProcessNamespace the Runner is pid 1, so its death tears down the pid namespace and reaps every VMM descendant. - The hostPath char-device route is non-functional, not merely worse posture ([INFERENCE] on cgroup-v2 device-controller behaviour, flagged as such and given a negative control in the spike). Scope is the core capability only. Cluster choice, node provisioning, and manifest delivery are operator concerns, named and deferred per the public/private boundary; the record cites the boundary convention rather than restating it. spike-findings.md records the verification's questions and negative controls BEFORE anyone runs them, so results cannot pick the questions. Each item states the control that must fail; a control that unexpectedly passes shrinks the contract. Mints DL-357. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…RIG-3336) Six findings from the review loop, all real. - The hostPath-over-emptyDir justification was wrong. emptyDir is pod-scoped and DOES survive a container restart, so the axis I contrasted did not distinguish them. Rejustified on outliving the POD: pod recreation and node reboot are when a previous pod's stale pidfile must still be reachable for orphan-reaping. - A DAC permission denial is EACCES, not EPERM (verified: errno 13). This is load-bearing rather than cosmetic, because the errno is what separates the two layers the record is at pains to distinguish -- cgroup device-controller denial is EPERM, filesystem denial is EACCES. Spike S3 now records the errno verbatim and states that seeing EPERM would mean the cgroup layer denied, confounding S3 with S2. - Guest RAM is charged to the pod's cgroup because the VMM is an ordinary child in that cgroup, NOT because of `--memory shared=on`. shared=on's consequence here is an accounting one: guest memory is shared mappings, so summing RSS across cloud-hypervisor, virtiofsd and the Runner double-counts it. Size against PSS or the configured guest total. - Fully-absent requests make the pod BestEffort, not Burstable. The eviction conclusion is unchanged and slightly stronger. - A probe-driven restart is the same node-wide session teardown as a rollout, but nothing rate-limits it the way maxUnavailable bounds a rollout. The contract now requires a conservative or absent liveness probe, and a terminationGracePeriodSeconds sized to the reap budget. - A node drain is equally session-affecting and was covered by nothing, since it is not a delivery mechanism. Named in the constraints, with the note that a fleet PDB bounds concurrent drains but cannot protect a pod on its own draining node. Gates on this tree: root:markdownlint rc=0, orion-ref-gate rc=0, ledger gate rc=0 (OK -- 319 rows, 133 headers). Co-authored-by: Matt Wilkinson <matt@rigel.build>
…h main (RIG-3336) CI caught a duplicate ledger id: main minted its own DL-357 (RIG-3561's pr_number wire-field row, #1037) while this branch was in review, so two rows claimed 357 and design-ledger-gate failed. My local gate had passed because the clone's main was 9 commits stale -- the id was genuinely free when I allocated it and taken by the time I pushed. A ledger id is only free at the moment the gate runs against current main, so it needs re-checking after any merge, not just at allocation. Renumbered to DL-358 (free on current main) and synced the record's three references: the Ledger-impact header, R6, and the tasks table. Merges current main, which also swaps the markdown gate from markdownlint-cli2 to rumdl (RIG-3663, #1094) -- verified via root:ci on the merged tree rather than the retired task. Gates on the merged tree: design-ledger-gate rc=0 (OK -- 320 rows, 133 headers), orion-ref-gate rc=0, root:ci rc=0. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…gency (RIG-3336) The record read as though containerizing the Runner might not work. It does not not work. The composition already boots on Linux with /dev/kvm -- the frozen microVM CI/dev enablement record runs KVM-backed boot tests as a required leg on GitHub Actions ubuntu-latest. What a pod adds over that is confinement: a cgroup device controller, a seccomp filter, a memory cgroup. So R8 asks which grants the confinement makes necessary, not whether microVMs run in containers. Every plausible outcome costs at most a wider pod spec this record already specifies -- a seccomp profile, a device plugin, a supplemental gid. The valuable outcome is a smaller contract, since a grant that proves inert drops out. The one result that would reopen the container-vs-host ruling is a requirement for a Linux capability or privileged: true, and no identified mechanism in the composition needs one. That is now stated as the low-probability tail rather than the expected case. Changes: - OQ-1 reframed from "is the zero-privilege pod sufficient" (load-bearing feasibility) to "which grants does confinement make necessary" (non-load-bearing spec narrowing). OQ-2 stays load-bearing -- the seccomp profile is the only item with a real cost attached -- but a needed profile is now named as a bounded cost, not a setback. - Approach, Global Constraints, Privilege shape, R8 and the spike header all cite the established KVM baseline instead of treating the boot as unproven. - S1 rescoped from "does the composition boot" to "does the confinement hold". - All 8 "contingent on R8" phrasings removed. Also supersedes a stale measurement: the frozen enablement record reads /dev/kvm as crw-rw-rw-, but this box measures crw-rw---- root:kvm (2026-09-12). The mode decides whether the supplementalGroups grant is required or inert, so R8 must record the node's own mode. Gates: design-ledger-gate rc=0 (320 rows, 133 headers), orion-ref-gate rc=0, root:ci rc=0. Co-authored-by: Matt Wilkinson <matt@rigel.build>
The reframe commit corrected design.md but left two things stale. The DL-358 ledger row still said the ruling was "explicitly CONTINGENT" on the spike and that the container-vs-host tradeoff "reopens" if it disappoints. That is the framing the reframe removed, sitting in the durable one-line summary a reader hits before the record. Rewrite it as a grant-set question, cite the KVM CI leg as the reason feasibility is settled, and state the capability/privileged tail as the only reopening outcome. Also restate the seccomp profile as specified-but-open (OQ-2) rather than flatly required. Prose referred to the privilege spike as R8 in 17 places; the task is R7 and no R8 exists, so every "R8 verifies it" pointed at nothing.
…age (RIG-3336) R1 said "a nix-built container image project ... mirroring the existing agent-image lane". That contradicts the private monorepo's frozen first-party-image-builds spec, which splits the two mechanisms by what the image's RUNTIME is rather than by what built the artifact: an image that IS a Nix environment (a CI step image, a dev/agent shell) earns nix2container, while a prebuilt application on a minimal base is a Dockerfile built by rootless BuildKit. The spec forecloses the exact inference R1 made — a first-party image does not earn the nix path "merely because Nix built it or it ships a compiled binary" — and rejects nix2container for app images on cost. The Runner is a prebuilt application: a static CGO_ENABLED=0 binary that exec's three userland binaries and runs no toolchain at runtime. agent-image sits on the other row by name, because that container's job is to be a toolchain; sharing the *-image/ directory shape is not sharing the mechanism. Distroless suits a managed multi-tenant cluster, but the KVM userland comes from nixpkgs and carries store-path interpreter/rpath references, so R1 must settle how those binaries run on a minimal base. R1's test cycle now asserts each carried binary EXECUTES, which a layer-contents check alone would miss. R2 gains the build mechanism and the digest-pinning contract: GHCR has no server-side tag immutability, so R3 pins repo@sha256, never a tag.
…n a container image (RIG-3720) R1 of the Runner containerization record. A Dockerfile on distroless, built by rootless BuildKit — the ruled mechanism for a prebuilt application. Nix still builds every carried artifact; it is the builder, never the runtime. The image carries the compass-runner binary, the KVM userland it exec's (cloud-hypervisor, virtiofsd, passt) and the three guest assets the microVM backend direct-boots, so a Runner pod needs no host toolchain. WHY DISTROLESS STATIC AND NOT :base. The carried binaries are dynamically linked but nix-closed: each names an absolute /nix/store interpreter and resolves every NEEDED library through its own RPATH, so the closure supplies its own ld.so and glibc and the base is never consulted for a library. That makes the base's libc dead weight. Measured, not assumed: with the closure staged all four binaries execute on distroless-static; with only cloud-hypervisor's own store path copied and its glibc omitted, the same exec fails "missing dynamic library" — the control proving the loader comes from the closure, not the base. Two consequences that are easy to get wrong, both encoded in the lane: - The staged context is the TRANSITIVE closure, not the five roots. A context missing the shared glibc yields an image whose binaries cannot exec at all. - The entrypoint is a RELATIVE symlink inside store/. An absolute symlink at the context root dangles on the build host, and BuildKit checksums context entries before any COPY runs, so it fails the build outright rather than resolving later inside the image. The realise-and-stage logic is TypeScript (the no-bash-gate task), split pure-core/thin-runner like the microvm-boot-test lane: build-core.ts holds the out-path parsing, the artifact->build-arg mapping and the closure-root selection, covered by 13 unit tests. The image build itself is runInCI:false — it needs a buildkitd, which no compass CI step carries until R2 — so the gate coverage is that suite, which is where a mapping drift is actually caught. Verified end-to-end on a rootless buildkitd: the lane builds the real image; its entrypoint runs as uid 65532; all four binaries execute in-image; passt resolves by name off PATH; all three guest assets match their host sizes byte for byte.
The Apple container runtime tier on macOS is arm64, and the compass-agent image is published linux/amd64 only, so that tier has no agent image to run. This records the multi-arch publish lane. Consumer-facing tags stay bare and become an OCI image index, so no consumer changes: every puller already relies on engine platform negotiation. Per-arch tags are internal immutable building blocks. Each arch builds natively on its own runner rather than under QEMU. The image closure dominates CI cost and emulating a nix build multiplies it; GitHub now offers free arm64 runners on public repositories. No guard is dropped to make the pipeline green. Immutability, two-tag coherence and the semver re-tag move to manifest-list digests, and the linux/amd64 tripwire becomes an exact platform-set assertion with per-member arch and os checks, which is a stricter contract than it replaces. The toolchain has never run on aarch64-linux, so a dispatch-only spike gates the lane rather than a claim that it works. Co-authored-by: Matt Wilkinson <matt@rigel.build>
T1 scoped the arm64 entrypoint work to the nodeModules FOD hash alone, but the native-addon copy block names linux-x64 three times and its modern/baseline split is an AVX2 concept with no arm64 analogue. An implementer following T1 as written would not have touched it. T1 now owns that edit, and the unverified part is narrowed to which variant names the arm64 package actually ships. The index immutability guard and the clean re-run after a mid-compose failure both assumed an OCI index recomposes to identical bytes. The spec does not canonicalize an index, and an unproven assumption sat inside a guard whose failure arm hard-fails against an immutable tag. It is now OQ-5, gated on the spike, with a fallback identity (assert the member digest set) that keeps the immutability property without depending on byte-stable serialization. Also hedge the nix2container claim where it is made rather than two sections away, and anchor the ledger row into the decision it records. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…root uid (RIG-3720) Review findings on the R1 lane. The load-bearing one: the image was NOT reproducible, contradicting R1's acceptance and a Dockerfile comment that claimed the COPY layer was "bit-identical to the staged tree". Two builds of identical inputs produced different manifest digests. Two independent causes, both measured rather than reasoned about: - cpSync defaults preserveTimestamps:false, so every staged file got a wall-clock mtime instead of the 1s nix normalises its store to. - BuildKit carries the context's mtimes into the layer tar regardless. Fixing only the first was NOT sufficient: the staged tree hashed bit-identical across runs while the `COPY store` layer digest still moved. The export needs rewrite-timestamp=true plus a fixed SOURCE_DATE_EPOCH. With both, two consecutive builds yield the same digest. The publish lane asserts exactly this, so the property is now real instead of asserted. Also from review: - Drop the RUNNER_BIN build-arg. It was declared and supplied but never consumed — the entrypoint became a staged symlink and the arg was left behind. The new drift test is what catches this class: it parses the Dockerfile's own ARG lines and asserts set-equality with what build-core supplies. Confirmed it fails when an ARG is renamed and passes when restored. - Add an explicit USER 65532:65532 rather than inheriting nonroot from an opaque base digest, so a future digest bump onto the root variant cannot silently produce a root-running container. - Assert the build host is x64. The platform is a manifest LABEL BuildKit applies without checking the staged files; today only flake.nix's systems list keeps them honest, which this lane never reads. - Narrow the build task's inputs from `**/*` to named tracked files: moon's native walker hashes contents directly and does not skip gitignored paths, so the glob would hash the multi-GB closure this task itself writes. - Cut the mechanism rationale from five restatements to one plus pointers, drop forward task references from source, and delete a tautological test that could not fail.
…G-3625) Both T1 edits need an aarch64 value only the spike runner can produce, so landing the seam alone would put a fakeSha256 placeholder and an unresolved copy block on main where nothing selects them until the arm64 lane exists. That is config that provably does nothing. T1 now lands with the measured hash and variant names in one change, or waits for the cutover. Co-authored-by: Matt Wilkinson <matt@rigel.build>
The CI `setup` job failed at "Generate the concern matrix", blocking every downstream leg. tools/ci-matrix requires each moon project to carry exactly one `ci-group.*` tag and fails loud otherwise — the fail-loud posture that lets moon own discovery instead of a stale YAML enumeration — and the new compass-runner-image project shipped with `tags: []`. ci-group.nix, with the sibling image projects: the real build closure is the nix one those legs already provision. The project contributes no CI task today (`build` is runInCI:false), so the tag places it in the graph rather than adding a leg. `moon ci` does not run the matrix generator, so a green local sweep said nothing about this. Run `bun run tools/ci-matrix/index.ts` when adding a project.
Another lane landed DL-364 on main while this PR was in review, so the row collided and the ledger gate failed in CI. Main is now at DL-365, making DL-366 the free id. Co-authored-by: Matt Wilkinson <matt@rigel.build>
The ledger row collided with another lane, so this branch needs main in its history for the ledger gate to see every id. Co-authored-by: Matt Wilkinson <matt@rigel.build>
…RIG-3603)
Compass UI chrome depends on 29 non-ASCII glyphs that neither brand face
covers, held up by a ~14 MB Unifont pin in the e2e toolchain. This record
adopts the dot-matrix `crispEdges` SVG technique DL-150/DL-199 already froze
and shipped in `StateDot`/`BadgeGlyph`, and sets the conditions for retiring
the pin.
It also splits `ActivityBarItem.icon: string`, which conflated two different
primitives: a fixed symbol from a closed set, and an agent handle's initial.
They split at the item — `GlyphTabItem` vs `AvatarTabItem` — because a
field-level union preserves exactly the confusion that made the field hard to
read.
Three findings that reshaped the plan, each grounded in the code:
- The glyph lane repaints pixels inside at least 5 of the 11 committed
baselines, so it recaptures at T6; the pin removal then recaptures again.
An earlier draft scheduled all recapture in the pin PR, which would have
shipped a PR that fails the visual gate with no step to fix it.
- `.term-body` renders through `var(--cx-font-ui)` — there is no separate
terminal font stack — and `agent.png` captures the `stub-data.ts` fixture
glyphs, so the pin cannot retire until those two strings are ASCII.
- The union breaks two reads, not one: `{tab.icon}` and `tab.agentId`. T3 and
T4 therefore land as a single commit; split, the first is red at typecheck.
Ledger-impact: adds DL-366.
Co-authored-by: Matt Wilkinson <matt@rigel.build>
|
Compass engineering docs preview: https://trunk-merge-pr-1164-ddebf2d4.compass-eng-docs.pages.dev Deployed from Changed pages: |
trunk-io
Bot
deleted the
trunk-merge/pr-1164/ddebf2d4-98fe-48e8-90fd-d84e204a8193
branch
September 12, 2026 21:53
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request was created and is being managed by Trunk Merge.
This pull request is based on the main branch at SHA b336129efc1aa5976dc7d4e52755acf1c20c8151.
See more details about each PR in the batch here:
When CI completes, this pull request will be closed automatically.
Pull Requests Being Tested
This pull request is testing a batch with the changes from pull requests 1164, 1167, and 1172 - batching documentation.
Dependencies
This pull request depends on the changes from pull request 1162.