HOMELAB-1683: feat(cockpit): live attach registry + go-live wiring (LD3.3) #588

Merged

aaron merged 2 commits from plane/HOMELAB-1683-attach-live into epic/LD3 2026-10-02 21:12:54 +00:00

Conversation 3 Commits 2 Files changed 13 +1484 -68

aaron commented 2026-10-02 20:39:29 +00:00

Owner

Copy link

What

The live attach registry + go-live wiring for LD3.3 (HOMELAB-1683), off epic/LD3 @ 76625d2. lab-director/cockpit/** Go only — no web/**, no manifests, no flags in the Deployment (those are the prod PR).

#579 shipped the attach bridge behind LD_ATTACH_ENABLED with a registry seam that refused everything, because the read-only RBAC it needs was not ratified. The package §4/§5 delta is operator-ratified (2026-10-02), so this slice builds the live implementation.

The live registry (attach/kube_registry.go, new)

Three hops, every one the controller's answer rather than a name this process composed:

SandboxClaim <id>   extensions.agents.x-k8s.io/v1beta1, ns LD_ATTACH_NAMESPACE
  .status.sandbox.name ─▶ Sandbox agents.x-k8s.io/v1beta1
      .status.selector ─▶ LIST pods by the controller's own selector
        container      ◀─ the template container declaring stdin+tty (pi)
        class          ◀─ .spec.podTemplate.metadata.labels[lab-director/class]
        ticket         ◀─ lab-director/ticket (claim label/annotation, then template)
        identity       ◀─ LD_HERDR_LABEL on the harness container (claim binding)
        hasTTY         ◀─ template stdin+tty AND the ratified class table

Group/version read out of the pinned CRDs (core/manifests/agent-sandbox/vendor/sandbox-with-extensions-v1.0.4.yaml), not docs prose. Confirmed against the live cluster (read-only): Sandbox.status.selector = agents.x-k8s.io/sandbox-name-hash=… is exactly the label the pod carries; pi has tty=true stdin=true; ld-claude-code-pool-*'s claude-code container has neither.

hasTTY is evidence AND policy. browser/firmware/meet-ai ship the same pi container with a terminal, and the package §1 refuses those classes — so the conservative answer is reported, and the info read and the refusal cannot disagree. claude-code is not in the harness table and its shipped template declares no stdin/tty, so it refuses (unknown) rather than silently inheriting a shell.

Failure shapes are preserved: nothing attachable under an id → ErrUnknownSandbox with the registry's Why (unbound claim / no pod / torn down); a Forbidden is not an unknown sandbox — it survives as an error naming the grant, which the bridge renders as registry_unwired (503), so nobody is taught to stop retrying a sandbox that is there.

Wiring

The enabled path builds the executor and the live registry, once each, from one client configuration (NewKubeClient, shared with the executor so the two cannot read different configs). The disabled path constructs neither, asserted as call counts — TestWireDisabledConstructsNoKubeClient untouched and green, plus a new TestWireDisabledConstructsNoRegistry. LD_ATTACH_NAMESPACE (default agent-sandbox-system) added; the registry refuses an empty namespace rather than defaulting, because a wrong namespace looks exactly like a fleet with no sandboxes.

Verification

go vet ./...                                  clean
go build ./...                                ok
go test -race -count=1 ./...                  207 pass (base 192), 0 fail
gofmt -l                                      clean
LD_ATTACH_ENABLED=false --check               OK attach enabled=false kube_client_constructed=false registry=not-applicable (disabled) namespace=agent-sandbox-system kubeconfig=in-cluster

Tests are envtest-free: the live registry is driven by fixture CRD objects through the ClusterReader seam (mapping, the per-class TTY table taken from the prod templates, ticket/identity sources and their order, every unknown-vs-cannot-look shape, ambiguity determinism, offline construction, and one end-to-end pass through Wire + Info). Against the live cluster (read-only, real kubeconfig, dev mode): /api/v1/attach/<a warm-pool Sandbox name>/info → 404 unknown_sandbox, audited — proving the live registry performs real API reads rather than reading a stub.

Go-live gap, reported not hidden

No object in the cluster carries the Plane ticket today: the spawn arm passes no labels to create_sandbox, so lab-director/ticket is absent and every attach is refused as sandbox_without_ticket (409, audited). The remedy is one line at the spawn call site (cmd/dispatcher/pipeline.go), dispatcher territory, deliberately not in this PR. Same for LD_HERDR_LABEL (recorded empty, never guessed). Documented in lab-director/cockpit/README.md.

Refusals remain fail-closed and audited end to end; no new credentials, no writes, no watch.

## What The live attach registry + go-live wiring for LD3.3 (HOMELAB-1683), off `epic/LD3` @ `76625d2`. `lab-director/cockpit/**` Go only — no `web/**`, no manifests, no flags in the Deployment (those are the prod PR). `#579` shipped the attach bridge behind `LD_ATTACH_ENABLED` with a registry seam that refused everything, because the read-only RBAC it needs was not ratified. The package §4/§5 delta is operator-ratified (2026-10-02), so this slice builds the live implementation. ## The live registry (`attach/kube_registry.go`, new) Three hops, every one the controller's answer rather than a name this process composed: ``` SandboxClaim <id> extensions.agents.x-k8s.io/v1beta1, ns LD_ATTACH_NAMESPACE .status.sandbox.name ─▶ Sandbox agents.x-k8s.io/v1beta1 .status.selector ─▶ LIST pods by the controller's own selector container ◀─ the template container declaring stdin+tty (pi) class ◀─ .spec.podTemplate.metadata.labels[lab-director/class] ticket ◀─ lab-director/ticket (claim label/annotation, then template) identity ◀─ LD_HERDR_LABEL on the harness container (claim binding) hasTTY ◀─ template stdin+tty AND the ratified class table ``` Group/version read out of the pinned CRDs (`core/manifests/agent-sandbox/vendor/sandbox-with-extensions-v1.0.4.yaml`), not docs prose. Confirmed against the live cluster (read-only): `Sandbox.status.selector` = `agents.x-k8s.io/sandbox-name-hash=…` is exactly the label the pod carries; `pi` has `tty=true stdin=true`; `ld-claude-code-pool-*`'s `claude-code` container has neither. **`hasTTY` is evidence AND policy.** browser/firmware/meet-ai ship the same `pi` container with a terminal, and the package §1 refuses those *classes* — so the conservative answer is reported, and the info read and the refusal cannot disagree. `claude-code` is not in the harness table and its shipped template declares no stdin/tty, so it refuses (unknown) rather than silently inheriting a shell. Failure shapes are preserved: nothing attachable under an id → `ErrUnknownSandbox` with the registry's `Why` (unbound claim / no pod / torn down); a `Forbidden` is **not** an unknown sandbox — it survives as an error naming the grant, which the bridge renders as `registry_unwired` (503), so nobody is taught to stop retrying a sandbox that is there. ## Wiring The enabled path builds the executor **and** the live registry, once each, from **one** client configuration (`NewKubeClient`, shared with the executor so the two cannot read different configs). The disabled path constructs neither, asserted as call counts — `TestWireDisabledConstructsNoKubeClient` untouched and green, plus a new `TestWireDisabledConstructsNoRegistry`. `LD_ATTACH_NAMESPACE` (default `agent-sandbox-system`) added; the registry refuses an empty namespace rather than defaulting, because a wrong namespace looks exactly like a fleet with no sandboxes. ## Verification ``` go vet ./... clean go build ./... ok go test -race -count=1 ./... 207 pass (base 192), 0 fail gofmt -l clean LD_ATTACH_ENABLED=false --check OK attach enabled=false kube_client_constructed=false registry=not-applicable (disabled) namespace=agent-sandbox-system kubeconfig=in-cluster ``` Tests are envtest-free: the live registry is driven by fixture CRD objects through the `ClusterReader` seam (mapping, the per-class TTY table taken from the prod templates, ticket/identity sources and their order, every unknown-vs-cannot-look shape, ambiguity determinism, offline construction, and one end-to-end pass through `Wire` + `Info`). Against the live cluster (read-only, real kubeconfig, dev mode): `/api/v1/attach/<a warm-pool Sandbox name>/info` → 404 `unknown_sandbox`, audited — proving the live registry performs real API reads rather than reading a stub. ## Go-live gap, reported not hidden No object in the cluster carries the Plane ticket today: the spawn arm passes no `labels` to `create_sandbox`, so `lab-director/ticket` is absent and every attach is refused as `sandbox_without_ticket` (409, audited). The remedy is one line at the spawn call site (`cmd/dispatcher/pipeline.go`), **dispatcher territory, deliberately not in this PR**. Same for `LD_HERDR_LABEL` (recorded empty, never guessed). Documented in `lab-director/cockpit/README.md`. Refusals remain fail-closed and audited end to end; no new credentials, no writes, no watch.

aaron added 1 commit 2026-10-02 20:39:29 +00:00

HOMELAB-1683: feat(cockpit): live attach registry + go-live wiring (LD3.3) ... 8d5eab2f8c

The attach bridge shipped in #579 behind LD_ATTACH_ENABLED with a registry seam
that refused everything, because the RBAC it needs was not ratified. The
ratification package §4/§5 (operator-ratified 2026-10-02) scopes exactly that
grant, so this slice builds the live implementation.

Registry (attach/kube_registry.go, new): read-only, three hops, every one the
controller's answer rather than a name this process composed.

  SandboxClaim <id>  extensions.agents.x-k8s.io/v1beta1 (by name: the sandbox id
                     IS the claim name the spawn arm returns)
    .status.sandbox.name -> Sandbox agents.x-k8s.io/v1beta1
      .status.selector   -> LIST pods by the controller's own selector
        container        <- the template container declaring stdin+tty (pi)
        class            <- .spec.podTemplate.labels[lab-director/class]
        ticket           <- lab-director/ticket (claim, then template)
        identity         <- LD_HERDR_LABEL on the harness container
        hasTTY           <- template stdin+tty AND the ratified class table

hasTTY is evidence AND policy: browser/firmware/meet-ai ship the same pi
container with a terminal and the package refuses those classes by class, so the
conservative answer is reported and the info read and the refusal cannot
disagree. claude-code is not in the harness table and its shipped template
declares no stdin/tty, so it refuses as unknown rather than silently inheriting
a shell.

Failures keep their shapes: an id with nothing attachable under it is
ErrUnknownSandbox with the registry's Why (unbound claim, no pod, torn down); a
Forbidden is NOT an unknown sandbox — it survives as an error naming the grant,
which the bridge renders as registry_unwired (503) so nobody is taught to stop
retrying a sandbox that is there.

Wire: the enabled path now builds the executor AND the live registry, once each,
from one client configuration (NewKubeClient, shared with the executor so the two
cannot read different configs). The disabled path constructs neither, asserted as
call counts (TestWireDisabledConstructsNoKubeClient, and the new
TestWireDisabledConstructsNoRegistry) — the inertness invariant is unchanged.

Config: LD_ATTACH_NAMESPACE (default agent-sandbox-system via
Attach.NamespaceOr). The registry refuses an empty namespace rather than
defaulting: a wrong namespace looks exactly like a fleet with no sandboxes.

Tests: envtest-free — the live registry is driven by fixture CRD objects through
the ClusterReader seam (claim -> Sandbox -> pod mapping, the per-class TTY table
taken from the prod templates, ticket/identity sources and their order, every
unknown-vs-cannot-look shape, two-pod determinism, offline construction, and one
end-to-end pass through Wire + Info). 192 -> 207 tests, -race green.

Go-live gap reported, not hidden: no object carries the Plane ticket today (the
spawn arm passes no labels to create_sandbox), so every attach refuses as
sandbox_without_ticket (409, audited) until the one-line spawn change lands in
dispatcher territory; same for LD_HERDR_LABEL.

aaron added 1 commit 2026-10-02 20:52:38 +00:00

HOMELAB-1683: fix(cockpit): keep the pinned registry sentence byte-identical (web parity guard) ... 07105c058f

CI run #1752's cockpit-test failed on the FRONTEND step: the landed web parity
guard pins two Go strings character for character —
web/src/lib/tests/fixtures.test.ts ("keeps the registry wiring chips identical to
bridge.registryWiring"; "the sentence it ships IS the backend's") and
attach-launcher.test.ts (the chip text), both read out of attach/bridge.go.

`unwiredRegistryReason` and `registryWiring` are restored to their landed bytes.
They describe an EXPLICITLY refusing registry (NewUnwiredRegistry), which is
still a real disposition, and the comment above them now says so plus why they
must not be reworded from this territory. Rewording them would have required
editing two files under web/**, which this slice is bound not to touch.

The stale part is handled where it is reachable instead: the live registry's own
failure (a Forbidden, an unreachable API) now answers with liveRegistryReason /
liveRegistryWiring — "apply prod/manifests/lab-director/cockpit-rbac.yaml +
the kube-apiserver egress rule" — which no fixture pins and which names the fix an
operator actually has. Covered by
TestLiveRegistryFailureSpeaksTheLiveReasonNotThePreRatificationOne (asserts the
live sentence, the absence of the pre-ratification one, and the chip list).

Verified: Go vet + `go test -race -count=1 ./...` green (208 pass); the web suite
run locally against the whole cockpit module (as CI does, PLANE_GO_PATH depth
replicated) — 26 files / 249 tests pass, parity guard collected its 17 tests,
svelte-check 0 errors 0 warnings.

aaron commented 2026-10-02 21:12:35 +00:00

Author

Owner

Copy link

Review 1/2 — safety & inertness lens

VERDICT: APPROVE-WITH-CONCERNS — 0 blockers.

Fresh context, head 07105c058f (post-dates neither review). The safety invariant holds: inert when the flag is off, read-only cluster surface, identity resolved from controller-owned objects, the 404-vs-503 split intact, and no sentinel/dispatcher file touched.

Confirmed

• Inertness: wire.go disabled branch returns before reading opts.NewExecutor/opts.NewRegistry; only production call sites of NewKubeExecutor/NewKubeRegistry/NewKubeClient are the seam defaults. CLI probe: flag off + LD_ATTACH_NAMESPACE set → exit 0, NO kube client constructed. Flag off + LD_ATTACH_KUBECONFIG → exit 2 (FaultIncompleteAttach, pre-existing).
• TestWireDisabledConstructsNoRegistry is load-bearing for the seam (mutation calling opts.NewRegistry on the disabled branch → FAIL). It cannot catch a disabled branch calling the concrete NewKubeRegistry directly — hardening suggestion only.
• Registry is READ-only: reader calls are exactly Get(sandboxClaimGVR), Get(sandboxGVR), List(podGVR); ClusterReader is Get/List only; no Create/Update/Patch/Delete/Watch. All remotecommand/NewSPDYExecutor hits are in kube.go (the Executor).
• Identity never from sandbox-supplied state: identityOf reads spec.podTemplate...env[LD_HERDR_LABEL], harnessContainer reads the template containers' tty/stdin, ticketOf reads claim/Sandbox metadata + spec.additionalPodMetadata; podFor returns only pod names — no pod.GetLabels() anywhere.
• Refusal matrix: NotFound/unbound/no-selector/no-pod → UnknownSandboxError → 404; Forbidden/other → 503 liveRegistryReason; no-TTY class refused; empty namespace → construction refusal.
• Sentinel/dispatcher: 13 files, all lab-director/cockpit/**; no sentinel/dispatcher path.
• Pinned registry sentence byte-identical to origin/epic/LD3 (web parity guard intact).

Concerns (no blockers)

1. bridge.go:56-58,64,66 — the 503 sentence says the grant is "sandboxclaims + sandboxes + pods … ratified in §4/§5", but §4 names only sandboxclaims + pods. Adjudicated below: the merged prod manifest PR #884 explicitly carries agents.x-k8s.io/sandboxes get,list in its "RBAC scope, as written" table, so the grant went through operator-merged review; the residual is the §4 prose, not the code.
2. Same sentence + README name cockpit-rbac.yaml/cockpit-ciliumnetworkpolicy.yaml. Adjudicated: both files exist on prod live (PR #884 merged, 40490c8).
3. wire_test.go hardening: assert inertness on the concrete path too (construction counter).
4. bridge.go:425-430 — the live reason is attached to any non-unknown registry error (can misdirect).
5. main.go:330 — kube_client_constructed=%t prints Enabled() (tautological).
6. kube.go/kube_registry.go — "one client configuration" is two independent NewKubeClient resolutions that agree by construction.

Full report: .rpiv/artifacts/build/ld3-1683-golive-review-1-out.md

## Review 1/2 — safety & inertness lens **VERDICT: APPROVE-WITH-CONCERNS** — 0 blockers. Fresh context, head `07105c058f` (post-dates neither review). The safety invariant holds: inert when the flag is off, read-only cluster surface, identity resolved from controller-owned objects, the 404-vs-503 split intact, and no sentinel/dispatcher file touched. **Confirmed** - Inertness: `wire.go` disabled branch returns before reading `opts.NewExecutor`/`opts.NewRegistry`; only production call sites of `NewKubeExecutor`/`NewKubeRegistry`/`NewKubeClient` are the seam defaults. CLI probe: flag off + `LD_ATTACH_NAMESPACE` set → exit 0, `NO kube client constructed`. Flag off + `LD_ATTACH_KUBECONFIG` → exit 2 (`FaultIncompleteAttach`, pre-existing). - `TestWireDisabledConstructsNoRegistry` is load-bearing for the seam (mutation calling `opts.NewRegistry` on the disabled branch → FAIL). It cannot catch a disabled branch calling the *concrete* `NewKubeRegistry` directly — hardening suggestion only. - Registry is READ-only: reader calls are exactly `Get(sandboxClaimGVR)`, `Get(sandboxGVR)`, `List(podGVR)`; `ClusterReader` is Get/List only; no Create/Update/Patch/Delete/Watch. All `remotecommand`/`NewSPDYExecutor` hits are in `kube.go` (the Executor). - Identity never from sandbox-supplied state: `identityOf` reads `spec.podTemplate...env[LD_HERDR_LABEL]`, `harnessContainer` reads the template containers' `tty`/`stdin`, `ticketOf` reads claim/Sandbox metadata + `spec.additionalPodMetadata`; `podFor` returns only pod *names* — no `pod.GetLabels()` anywhere. - Refusal matrix: NotFound/unbound/no-selector/no-pod → `UnknownSandboxError` → 404; Forbidden/other → 503 `liveRegistryReason`; no-TTY class refused; empty namespace → construction refusal. - Sentinel/dispatcher: 13 files, all `lab-director/cockpit/**`; no sentinel/dispatcher path. - Pinned registry sentence byte-identical to `origin/epic/LD3` (web parity guard intact). **Concerns (no blockers)** 1. `bridge.go:56-58,64,66` — the 503 sentence says the grant is "sandboxclaims + **sandboxes** + pods … ratified in §4/§5", but §4 names only `sandboxclaims` + `pods`. *Adjudicated below: the merged prod manifest PR #884 explicitly carries `agents.x-k8s.io/sandboxes get,list` in its "RBAC scope, as written" table, so the grant went through operator-merged review; the residual is the §4 prose, not the code.* 2. Same sentence + README name `cockpit-rbac.yaml`/`cockpit-ciliumnetworkpolicy.yaml`. *Adjudicated: both files exist on prod `live` (PR #884 merged, `40490c8`).* 3. `wire_test.go` hardening: assert inertness on the concrete path too (construction counter). 4. `bridge.go:425-430` — the live reason is attached to any non-unknown registry error (can misdirect). 5. `main.go:330` — `kube_client_constructed=%t` prints `Enabled()` (tautological). 6. `kube.go`/`kube_registry.go` — "one client configuration" is two independent `NewKubeClient` resolutions that agree by construction. Full report: `.rpiv/artifacts/build/ld3-1683-golive-review-1-out.md`

aaron commented 2026-10-02 21:12:41 +00:00

Author

Owner

Copy link

Review 2/2 — correctness lens

VERDICT: APPROVE-WITH-CONCERNS — 0 blockers.

Fresh context, head 07105c058f. The three-hop CRD mapping is correct against the actual install, the class→TTY table matches the shipped templates, no test is a tautology, and go vet/gofmt/go test -race are clean at the head.

Confirmed

• CRD mapping: against the pinned CRDs — sandboxclaims.extensions.agents.x-k8s.io/v1beta1 (Namespaced, printer column .status.sandbox.name), sandboxes.agents.x-k8s.io/v1beta1 (Namespaced); status.selector is type: string; spec.podTemplate.spec.containers[] carries tty/stdin booleans and env[].{name,value}; spec.podTemplate.metadata.labels and claim spec.additionalPodMetadata present. Independently re-checked against the live CRDs (v1beta1 / Namespaced).
• Forbidden is not an unknown sandbox: Resolve branches only on apierrors.IsNotFound; every other error is wrapped.
• Disabled path constructs neither (call-count assertion + live --check probe).
• Web parity fix is load-bearing: 8d5eab2 had rewritten the pinned sentence and CI run #1752 failed cockpit-test; 07105c0 (#1756) is all-jobs green.
• Tests: 208 top-level PASS (+1 pre-existing SKIP, 0 FAIL) at head; base is 192. Every new registry test reviewed individually — each can fail on a wrong mapping, table entry, precedence, error class, or pod pick. No tautologies.

Class-map adjudication (explicit): light/build-test → TTY; browser/firmware/meet-ai → refused by class per §1 (their templates DO ship the terminal pi container — sandbox-*-template.yaml:242-243/283-284); claude-code → no table entry → honest refusal (its shipped template declares no stdin/tty, and spawn binds only five classes). The table is exhaustive over the declared class vocabulary and correct. The composite HasTTY = template-tty AND class-table is the right choice: /info and the refusal cannot disagree.

Concerns (no blockers)

1. kube_registry.go:236-238 + bridge.go:424-430 — a malformed Sandbox (containers: [], schema-reachable, no minItems) is diagnosed as a missing RBAC grant (503 + apply-the-RBAC chips). Fail-closed and audited, but misdirects the operator; the code comment claims this bucket is only "a Forbidden, a timeout".
2. kube_registry.go unstructured.Nested* errors silently discarded — a wrong-typed status.selector/status.sandbox.name reads as a normal lifecycle state (404 "not created yet"). Latent (CRD types both string).
3. attach.go:208-210,219,230-232 + bridge.go:447-450 — the class-refusal prose says browser/firmware/meet-ai "have no TTY" while the substrate declares one in the same PR. The reason is policy, not absence. Behaviour unaffected; the sentence is pinned in a web fixture so it cannot be reworded in this territory.
4. UnknownSandboxError.Why never reaches the wire: it travels in Refusal.Detail, and the /info body is the Info struct which has no detail field — the five 404 cases collapse to one sentence. Visible only in the ledger.
5. attach.go:168-171 — Sandbox.HasTTY's doc (evidence only) contradicts kube_registry.go:243-260 (evidence AND policy).
6. PR body says "207 pass"; head is 208 (the final commit added one test). Doc drift, no inflation.

Full report: .rpiv/artifacts/build/ld3-1683-golive-review-2-out.md

## Review 2/2 — correctness lens **VERDICT: APPROVE-WITH-CONCERNS** — 0 blockers. Fresh context, head `07105c058f`. The three-hop CRD mapping is correct against the actual install, the class→TTY table matches the shipped templates, no test is a tautology, and `go vet`/`gofmt`/`go test -race` are clean at the head. **Confirmed** - CRD mapping: against the pinned CRDs — `sandboxclaims.extensions.agents.x-k8s.io/v1beta1` (Namespaced, printer column `.status.sandbox.name`), `sandboxes.agents.x-k8s.io/v1beta1` (Namespaced); `status.selector` is `type: string`; `spec.podTemplate.spec.containers[]` carries `tty`/`stdin` booleans and `env[].{name,value}`; `spec.podTemplate.metadata.labels` and claim `spec.additionalPodMetadata` present. Independently re-checked against the live CRDs (`v1beta1 / Namespaced`). - `Forbidden` is not an unknown sandbox: `Resolve` branches only on `apierrors.IsNotFound`; every other error is wrapped. - Disabled path constructs neither (call-count assertion + live `--check` probe). - Web parity fix is load-bearing: `8d5eab2` had rewritten the pinned sentence and CI run #1752 failed `cockpit-test`; `07105c0` (#1756) is all-jobs green. - Tests: 208 top-level PASS (+1 pre-existing SKIP, 0 FAIL) at head; base is 192. Every new registry test reviewed individually — each can fail on a wrong mapping, table entry, precedence, error class, or pod pick. No tautologies. **Class-map adjudication (explicit):** `light`/`build-test` → TTY; `browser`/`firmware`/`meet-ai` → refused *by class per §1* (their templates DO ship the terminal `pi` container — `sandbox-*-template.yaml:242-243`/`283-284`); `claude-code` → no table entry → honest refusal (its shipped template declares no stdin/tty, and `spawn` binds only five classes). The table is exhaustive over the declared class vocabulary and correct. The composite `HasTTY = template-tty AND class-table` is the right choice: `/info` and the refusal cannot disagree. **Concerns (no blockers)** 1. `kube_registry.go:236-238` + `bridge.go:424-430` — a *malformed* Sandbox (`containers: []`, schema-reachable, no `minItems`) is diagnosed as a *missing RBAC grant* (503 + apply-the-RBAC chips). Fail-closed and audited, but misdirects the operator; the code comment claims this bucket is only "a Forbidden, a timeout". 2. `kube_registry.go` `unstructured.Nested*` errors silently discarded — a wrong-typed `status.selector`/`status.sandbox.name` reads as a normal lifecycle state (404 "not created yet"). Latent (CRD types both `string`). 3. `attach.go:208-210,219,230-232` + `bridge.go:447-450` — the class-refusal *prose* says browser/firmware/meet-ai "have no TTY" while the substrate declares one in the same PR. The reason is policy, not absence. Behaviour unaffected; the sentence is pinned in a web fixture so it cannot be reworded in this territory. 4. `UnknownSandboxError.Why` never reaches the wire: it travels in `Refusal.Detail`, and the `/info` body is the `Info` struct which has no detail field — the five 404 cases collapse to one sentence. Visible only in the ledger. 5. `attach.go:168-171` — `Sandbox.HasTTY`'s doc (evidence only) contradicts `kube_registry.go:243-260` (evidence AND policy). 6. PR body says "207 pass"; head is 208 (the final commit added one test). Doc drift, no inflation. Full report: `.rpiv/artifacts/build/ld3-1683-golive-review-2-out.md`

aaron commented 2026-10-02 21:12:47 +00:00

Author

Owner

Copy link

Landing adjudication — deviations ruled, concerns classified

Both independent reviews returned APPROVE-WITH-CONCERNS, 0 blockers at head 07105c058f. All findings are diagnostic-prose/doc-accuracy or enabled-mode hardening; none breaks flag-off inertness, the read-only registry, identity provenance, the refusal matrix, or scope.

Deviation 1 — class→TTY table vs the templates (from manifest evidence). Ruled CORRECT. The composed rule is HasTTY = template declares stdin+tty AND the ratified class table admits it. Verified against the shipped prod templates: light/build-test declare the terminal pi container; browser/firmware/meet-ai also ship the same pi container with stdin: true/tty: true yet are refused by class under §1 — so the conservative answer is reported and the /info read and the socket refusal cannot disagree. claude-code has no table entry and its template declares no tty (sandbox-claude-code-template.yaml:335, single claude-code container), and spawn binds only five classes — the honest answer is an unknown-class refusal, not a silent shell. The §1 prose naming claude-code as a TTY class is the stale artifact, not the code; the builder flagged it.

Deviation 2 — namespaced Role vs ClusterRole. Ruled CORRECT, same scope. §4 says "ClusterRole"; every kind is scope: Namespaced in the pinned CRDs, the fleet is one namespace, and the merged prod manifest (#884) implements a namespaced Role + RoleBinding with exactly four rules and nothing wider (no watch, no sandboxtemplates/sandboxwarmpools, no write, no pods/attach/pods/log/pods/portforward). The in-cluster precedent (agent-sandbox-mcp/rbac.yaml) is a namespaced Role for the same blast-radius reason. A ClusterRole would be strictly wider with nothing to show for it.

Deviation 3 — sandboxes read not named in §4. Resolved. Review 1 flagged that the live registry GETs agents.x-k8s.io/sandboxes while §4 names only sandboxclaims + pods. The hop is genuinely required (claim.status.sandbox.name → Sandbox → status.selector → pod), read-only, and fail-closed; and the merged prod manifest PR #884 lists agents.x-k8s.io/sandboxes get,list explicitly in its "RBAC scope, as written" table, so the grant passed operator-merged review. Residual: §4's prose. Likewise the 503 sentence's pointers to cockpit-rbac.yaml/cockpit-ciliumnetworkpolicy.yaml are accurate — both files are on prod live.

Deviation 4 — sentinel NOT armed. Confirmed out of scope, correctly. This PR does not touch the sentinel or the dispatcher: 13 files, all under lab-director/cockpit/**, no cmd/dispatcher/**, no sentinel path. The S1 ruling stays parked as HOMELAB-1874.

Concerns are posted above for the follow-up slices (live-error typing, Why on the wire, class-refusal prose, --check column). None blocks the flag-off merge.

## Landing adjudication — deviations ruled, concerns classified Both independent reviews returned **APPROVE-WITH-CONCERNS, 0 blockers** at head `07105c058f`. All findings are diagnostic-prose/doc-accuracy or enabled-mode hardening; none breaks flag-off inertness, the read-only registry, identity provenance, the refusal matrix, or scope. **Deviation 1 — class→TTY table vs the templates (from manifest evidence).** *Ruled CORRECT.* The composed rule is `HasTTY = template declares stdin+tty AND the ratified class table admits it`. Verified against the shipped prod templates: `light`/`build-test` declare the terminal `pi` container; `browser`/`firmware`/`meet-ai` **also** ship the same `pi` container with `stdin: true`/`tty: true` yet are refused **by class** under §1 — so the conservative answer is reported and the `/info` read and the socket refusal cannot disagree. `claude-code` has no table entry and its template declares no tty (`sandbox-claude-code-template.yaml:335`, single `claude-code` container), and `spawn` binds only five classes — the honest answer is an unknown-class refusal, not a silent shell. The §1 prose naming `claude-code` as a TTY class is the stale artifact, not the code; the builder flagged it. **Deviation 2 — namespaced Role vs ClusterRole.** *Ruled CORRECT, same scope.* §4 says "ClusterRole"; every kind is `scope: Namespaced` in the pinned CRDs, the fleet is one namespace, and the merged prod manifest (#884) implements a namespaced Role + RoleBinding with exactly four rules and nothing wider (no `watch`, no `sandboxtemplates`/`sandboxwarmpools`, no write, no `pods/attach`/`pods/log`/`pods/portforward`). The in-cluster precedent (`agent-sandbox-mcp/rbac.yaml`) is a namespaced Role for the same blast-radius reason. A ClusterRole would be strictly wider with nothing to show for it. **Deviation 3 — sandboxes read not named in §4.** *Resolved.* Review 1 flagged that the live registry GETs `agents.x-k8s.io/sandboxes` while §4 names only `sandboxclaims` + `pods`. The hop is genuinely required (`claim.status.sandbox.name` → Sandbox → `status.selector` → pod), read-only, and fail-closed; and the merged prod manifest PR #884 lists `agents.x-k8s.io/sandboxes get,list` explicitly in its "RBAC scope, as written" table, so the grant passed operator-merged review. Residual: §4's prose. Likewise the 503 sentence's pointers to `cockpit-rbac.yaml`/`cockpit-ciliumnetworkpolicy.yaml` are accurate — both files are on prod `live`. **Deviation 4 — sentinel NOT armed.** *Confirmed out of scope, correctly.* This PR does not touch the sentinel or the dispatcher: 13 files, all under `lab-director/cockpit/**`, no `cmd/dispatcher/**`, no sentinel path. The S1 ruling stays parked as HOMELAB-1874. Concerns are posted above for the follow-up slices (live-error typing, `Why` on the wire, class-refusal prose, `--check` column). None blocks the flag-off merge.

aaron merged commit e9e7a6826b into epic/LD3 2026-10-02 21:12:54 +00:00

aaron referenced this pull request from a commit 2026-10-02 21:12:55 +00:00

Merge pull request 'HOMELAB-1683: feat(cockpit): live attach registry + go-live wiring (LD3.3)' (#588) from plane/HOMELAB-1683-attach-live into epic/LD3

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

operator-review

Requires human operator review before merge — dangerous class (RBAC, secrets, network policy, Terraform) or explicitly gated. Never auto-merge.

No labels

operator-review

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/infra-core!588

No description provided.