refactor(cluster,evals): one Kubernetes call family, one diagnostic bound, one image-producer fact - #982
Merged
Merged
Conversation
…ound, one image-producer fact Every Kubernetes call in the simulator now goes through the bounded `kubernetesCall` family, so PR #978's stated invariant holds for the society API's workload, secret, and sandbox calls too. The society seam keeps `ClusterError` as its public error type through one adapter. The evals-side diagnostic bound was structurally dead — the submitter already publishes at most 8192 UTF-8 bytes, so a naive 8192-code-unit slice could only split surrogate pairs — and is gone. The submitter's byte-aware bound is the single authority; the worker's two layers now name their units. `ledgerAllocationFailed` and `runInfrastructureFailed` were the same function twice; they merge into one constructor keyed on the summary `_tag`, and the sequential string-compare dispatch reduces to one. The image-producer fact is one descriptor with the controller path written once, and `EvaluationImageKey` is derived from it. Also: the invisible-workspace-file list is computed once at definition time; the `McpServer` structural narrow is one exported matcher rather than three `"url" in server` sites; the worker-roll visibility query quotes the operator-supplied task queue and stops interpolating a constant status; `INSTALL_ORDER` derives its membership from `RunWorkerManifests` so a new object fails compile; and the cluster.sh prelude assertion checks definition and use rather than an occurrence count. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
A reviewed cleanup inventory over
packages/simulator/src/clusterandpackages/evals/src. No behaviour is added; the changes collapse facts that were stated more than once and remove one dead bound.The Kubernetes call families merge (worst offender)
kubernetes/calls.tscarried two parallel wrapper/error families: the society API (request/ignoreAbsent/ClusterError), which was unbounded, and run-control (kubernetesCall/attemptUnlessAbsent/KubernetesCallFailed), bounded viaEffect.timeoutFail. They now share one family, so PR #978's stated invariant — bound every Kubernetes call — is true for the workload, secret, and sandbox calls too, and there is one 404-is-absent rule rather than two spellings of it.ClusterErrorstays the society seam's declared error because it is public API (SimulatorRunFailure,Society, therun-specshape canary). The two families reduce to one wrapper plus oneEffect.mapErroradapter at that seam, rather than to one error type. Society-call failure text now readscreate agent sandbox failed (Kubernetes 409)instead ofcreate agent sandbox: <cause message>, and an unanswered call readsdid not answer in time.The controller diagnostic was bounded four times in three units
evals/src/submission.tsbounded it to 8192 code units after decode. That bound was structurally dead: the submitter publishes at most 8192 UTF-8 bytes, and a string of ≤8192 bytes is ≤8192 code units, so the slice was the identity — while still being able to split a surrogate pair. Deleted, along with the comment that justified it.submit.ts'sboundedDiagnostic— the byte-aware one — is left as the single authority.watch.ts's two layers now name their units (RETAINED_DIAGNOSTIC_CHARACTERS,FETCHED_LOG_BYTES) instead of one constant doing double duty as characters and aslimitBytes * 2.The submission test that asserted trimming now asserts what actually matters there: an over-long diagnostic does not cost the run its receipt.
cli.ts: two constructors that were oneledgerAllocationFailedandrunInfrastructureFailedwere the same function twice — two constructors, two optional diagnostic parameters, two??constants. They merge intoinfrastructureFailed, keyed on the summary_tag, andcompleteSubmission's sequential string-compare dispatch becomes a single check on the discriminant. Both old names were exported only for tests; the tests now go through the merged surface.cli.ts: one producer descriptor per imageThe image-producer fact was spread across four names with the controller script path written twice. There is now one descriptor with the two controller keys aliased to one constant, and
EvaluationImageKeyis derived from it rather than re-enumerated. The error text still names the producing script, and the existing canary that asserts the named script exists on disk is untouched.Smaller items
openclaw/runtime.ts—invisibleWorkspaceFiles()ran once at definition time inside the reachability guard and again per render. It is computed once and carried on the settings snapshot.agents/workspace.ts— theMcpServerstructural union was re-narrowed by"url" in serverat three sites. One exportedisHttpMcpServermatcher now owns the narrow. No_tagwas added: bare{ name, url }servers are constructed by downstream callers and must keep working.cluster/temporal.ts— the worker-roll visibility query interpolated the operator-suppliedtaskQueueunescaped. Quotes in the value are doubled, with a test for it, and the never-varying execution status is one constant clause instead of an interpolation pretending to be dynamic.cluster/install.ts—INSTALL_ORDERre-enumeratedkeyof RunWorkerManifests. It now derives from a record checked exhaustive against that type, so a new control-plane object fails to compile until it is given a position. The ordering comment is load-bearing and stays.gke/profile.test.mjs— the prelude assertion counted source-text occurrences (begin_cluster_session× 3). It now checks that the function is defined and invoked, which is the property that was meant.cluster.shbash-quirk notes, and the load-bearing invariants ininstall.ts,calls.ts,openclaw/runtime.ts,workspace.ts,nanoclaw/runtime.ts, andtemporal.tsare untouched.Verification
corepack pnpm check,corepack pnpm test, andcorepack pnpm nx run-many -t docs:generateall pass with the tree clean afterwards. Full suite green on the rerun (no flakes). The regression guards held untouched: same-image resubmission stays a no-op, the worker-roll refuse test, the MCP url render tests, and the workspace reachability guard tests.Note: GitHub Actions is in a major outage as of this writing, so CI has not run on this branch. Please wait for a green CI before merging.
🤖 Generated with Claude Code