Skip to content

Trigger the control-plane update: system-update job + admin endpoint - #386

Merged
onel merged 2 commits into
devfrom
backend/381-system-update-job
Aug 11, 2026
Merged

Trigger the control-plane update: system-update job + admin endpoint#386
onel merged 2 commits into
devfrom
backend/381-system-update-job

Conversation

@onel

@onel onel commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Closes #381.

Nothing on a box could start a control-plane update. #385 shipped the transaction (internal/hostagent/cpupdate) with no caller. This is the trigger.

What this adds

host-agent — the first job on the socket.

  • POST /v1/jobs/system-update with {brain_image?, ui_image?}202 {job_id, kind, status, started_at}. 400 when both refs are empty, 501 when no updater is wired (the fake binary).
  • GET /v1/jobs/{id} → the record, with error {code, message} and result {brain_changed, ui_changed, reverted, failure_mode, revert_error} once it ends.
  • cpupdate.Runner is the provider behind the new hostagent.SystemUpdater seam. cmd/host-agent-real wires it from the same brainLaunchConfig it uses to launch the brain at boot, so an updated brain stays identical to a first-boot one except for the image.

Brain — the admin route.

  • POST /api/v1/system/update (admin only) → the host job id. GET /api/v1/system/update/{job_id} polls it.
  • hostclient.StartSystemUpdate / Job, with typed sentinels (ErrUpdateInProgress, ErrJobNotFound) so the brain answers 409 and 404 rather than a flat 502.
  • New elevation-class audit action system.update.

Design calls, and why

The job surface is the minimum, not Pattern B's framework. The spec describes a kind registry with typed attributes, resource-class serialization, a cross-class dangerous lock, cancel, and an SSE log with 256 KB replay. There is exactly one job kind, so all of that would be an abstraction with a single consumer — CLAUDE.md # Go code discipline says not to build it. What is built: an in-memory record, a 30-minute MaxDuration, and one global lock (Dangerous: true realized for one kind). BRAIN_HOST_PROTOCOL.md # Pattern B now carries an "as built" block saying exactly what exists and what does not, so the spec stops claiming a framework the code does not have. A second job kind — enroll-drive — is what would make us generalize, and it is the case that actually needs resource classes and queueing.

A second update is refused with 409, not queued. Queueing is right when two different dangerous ops collide. The same op arriving twice is almost always a double click, and an admin who clicks Update twice wants one update.

The run gets its own context, not the request's. The caller is the brain — which this update may be about to replace. An inherited context would let the brain's own recreate cancel the transaction that was recreating it, mid-flight. The only bound is MaxDuration; past it the context is cancelled and cpupdate rolls back on a context of its own.

No stalled status. The spec reserves it for "we are not sure". Past MaxDuration this job is cancelled and reverted, so the outcome is known: failed with error.code = "job-timeout", which names both what happened and which rule fired.

The job id is host-agent's, not wrapped in a brain-side job. The brain has its own Pattern B registry and deliberately does not use it here: a brain-side record dies the moment the update recreates the brain. host-agent is the process that stays up, so a poll still answers after the brain has been replaced.

Audit success=true means "the update started", not "it worked". The brain cannot audit the outcome of an operation that replaces the brain; the outcome lives on the job record.

The target is two explicit image refs in the request — no release manifest, no cloud call. The box↔cloud credential is Tier-1 undesigned (NEXT.md), and the whole updater is testable and shippable without it.

Known gaps

  • Nothing picks the refs. No release manifest, no cloud target poll, no hourly check, no "update available" notification. UPDATES.md # 3 steps 1–2 do not exist.
  • No dashboard surface. API only; the regenerated client types are the sole UI-side change.
  • No retry, and no three-strikes pin (# 3 step 5). One attempt per request.
  • Job records are in memory and never evicted. Lost on host-agent restart (matching "Dangerous: crash mid-flight = no auto-resume"), so an admin who loses the id has no "how did the last update go" read.
  • No elevation re-prompt. Admin-only per the issue. USERS_AND_GROUPS.md asks for a 5-minute re-prompt on destructive UI ops, and replacing the control plane arguably qualifies — flagged as a question rather than decided here.
  • Fakes on both sides; no real box. The host tests drive a stub updater; internal/hostclient/jobs_test.go runs the real agent handler over a real UNIX socket. A real update on a booted box is Prove the updater on a booted box in CI: real update and real revert in the cloud-image lane #382 and stays the biggest gap in this stream.
  • cmd/host-agent-real was not compiled locally (pre-existing PAM cgo issue on this machine). Type-checked with CGO_ENABLED=0 go build ./cmd/host-agent-real/; CI does the real build.

Testing

New tests in four packages, all additions — git diff --numstat -- '*_test.go' shows no deletions or rewrites.

Mutation-checked (break the code, confirm the test fails, restore):

  • global lock removed → the 409 test fails
  • lock never released → the "second update after the first finished" test fails
  • MaxDuration bound removed → the timeout test fails (job never leaves running)
  • timeout not distinguished from failure → the job-timeout assertion fails
  • run inherits the request context → the client-hangup test fails
  • ErrNothingToDo treated as an error → the no-op test fails
  • revert error dropped from the wire result → the revert-error test fails
  • 403 not audited → the member test fails
  • 409 not mapped → the conflict test fails
  • control-char guard neutered → the malformed-ref test fails

Gates: make fmt-check, make openapi-check, make test-nopam, go vet over every package but the PAM one, and make check-web — all green. (make check cannot run on this machine for two reasons that pre-date this branch: vet walks gitignored mkosi leftovers, and cgo PAM does not resolve C.RTLD_NEXT.)

Review round 2 (commit 2)

Block — this PR also fixes a real bug in the merged transaction. cpupdate.Apply's snapshot-failure branch put the old brain container back with d.Run(ctx, ...) on the caller's context. Every other recovery path routes through revert(), which detaches with context.WithoutCancel and its own budget. Nothing called Apply before this branch, so nothing had ever handed it a context that could die — this branch hands it exactly that (context.WithTimeout(context.Background(), systemUpdateMaxDuration)) plus the production Docker whose exec.CommandContext honours it. A MaxDuration expiring during the snapshot, after the old container had already been removed, would have made the recovery docker run fail instantly and left the box with no brain container at all.

The detach is now one helper, detached(ctx), used by both recovery paths — the drift between them was the bug. New TestSnapshotFailureRestoresBrainOnDeadContext drives the branch with a context that dies at the container removal and asserts the old brain is started again; mutation-checked (with the fix reverted it fails with run:malmo-brain:latest: context canceled, which is only meaningful because the Docker fake honours a dead context).

Note — the two validation 422s no longer audit. CLAUDE.md # Go code discipline: "Pure reads and validation 422s don't audit." The 403, 409, 502, and the accepted start are unchanged. Both tests now assert the audit table stays empty, and that assertion is mutation-checked too.

The progress entry records the bug and the fix in its own section, referencing (not editing) control-plane-update-transaction.md. Gates re-run: fmt-check, openapi-check, test-nopam, non-PAM go vet, plus -race over the touched packages. git diff --numstat -- '*_test.go' shows one new test file and one comment/assertion edit — no deletions.

https://claude.ai/code/session_01DpJrjCXiQygMgNsw2AqH25

Nothing could start an update: the transaction (#380) shipped with no
caller. This adds the trigger.

host-agent gains its first job: POST /v1/jobs/system-update takes two
explicit image refs and answers 202 with a job id; GET /v1/jobs/{id}
polls it. The surface is the minimum Pattern B needs for one dangerous
kind - an in-memory record, a 30-minute MaxDuration, and one global
lock - not the full framework. There is one job kind, so a registry with
typed attributes, resource classes, cancel, and an SSE log would be an
abstraction with a single consumer. A second kind (enroll-drive) is what
would make us build it, and the spec now says so plainly.

The brain gains POST /api/v1/system/update (admin only) and
GET /api/v1/system/update/{job_id}. The job id is host-agent's, not
wrapped in a brain-side job: a brain-side record would die the moment
the update recreates the brain. Starting an update is elevation-class,
so it audits the start and every refusal (403, 422, 409, 502).

A second update while one runs is refused with 409, not queued. The run
uses its own context, not the request's - the caller is the brain, which
this update may replace.

Closes #381.

Claude-Session: https://claude.ai/code/session_01DpJrjCXiQygMgNsw2AqH25
@greptile-apps

greptile-apps Bot commented Aug 11, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
internal/hostagent/cpupdate/update.go Snapshot-failure recovery now restarts the old brain using the same detached, bounded context as other rollback paths.
internal/hostagent/cpupdate/recovery_test.go Adds a focused regression test proving that cancellation during container removal does not prevent restoration after snapshot failure.
internal/hostagent/jobs.go Implements the in-memory system-update job lifecycle, global exclusion lock, timeout handling, and result recording.
internal/api/systemupdate.go Adds admin-only start and polling endpoints with validation, typed host error mapping, DTO conversion, and audit recording.
internal/hostclient/hostclient.go Adds typed client operations for starting and polling host-agent update jobs.
cmd/host-agent-real/main.go Wires the production updater from the same brain launch configuration used during boot.
internal/protocol/host.go Defines the shared request, job, error, and update-result wire contracts.
api/openapi.yaml Publishes the new system-update request, job, result, and route schemas.

Reviews (2): Last reviewed commit: "Detach the snapshot-failure recovery, an..." | Re-trigger Greptile

Comment on lines +91 to +98
Docker: cpupdate.NewCLIDocker(),
Prober: cpupdate.HTTPProber{},
Base: cpupdate.Options{
ControlPlaneDir: brainCfg.ControlPlaneDir,
BrainCfg: brainCfg,
SnapshotRoot: filepath.Join(brainCfg.DataDir, "brain-snapshots"),
},
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Cancelled snapshot recovery context

When the job deadline expires while the old brain has been removed and its database snapshot is failing, the newly wired updater attempts to restart the old brain using the cancelled job context, causing that restart to fail and leaving the box without a running brain until manual recovery.

Knowledge Base Used: Brain ↔ host-agent protocol

Review of #386 found one Block and one Note.

Block: cpupdate.Apply put the old brain container back on the caller's
context when the snapshot step failed. Every other recovery path routes
through revert(), which detaches with context.WithoutCancel and its own
budget. Nothing called Apply before this branch, so nothing had ever
passed it a context that could die; this branch passes exactly that - a
30-minute MaxDuration on a Docker that honours it. A deadline expiring
mid-snapshot would have left the box with no brain container at all.

The detach is now one helper, detached(ctx), used by both recovery
paths, because the drift between them was the bug. New regression test
drives the branch with a context that dies at the container removal and
asserts the old brain is started again; it fails with the fix reverted.

Note: the two validation 422s no longer audit. CLAUDE.md is explicit
that pure reads and validation 422s don't audit. The 403, 409, 502, and
the accepted start are unchanged.

Claude-Session: https://claude.ai/code/session_01DpJrjCXiQygMgNsw2AqH25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trigger the control-plane update: system-update job + admin endpoint

1 participant