Skip to content

Wire compile_pool_ensure into fleet convergence: the managed slice is modeled but unreachable - #9141

Closed
gunbai-bot[bot] wants to merge 11 commits into
mainfrom
session/stern-boar-129
Closed

gunbai-bot[bot] wants to merge 11 commits into
mainfrom
session/stern-boar-129

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session stern-boar-129.
Pushing to session/stern-boar-129 advances this PR.

Worker attestation

Before flipping this PR to ready for review, confirm each item:

  • Title describes the change (not the session id or branch).
  • PR body summarises what and why (replace the TODO below).
  • Tests run: name the command (e.g. npm test, cargo test) and the result.
  • If this closes a work item, the body contains a Closes #N directive.
  • No commits on this branch are surprises (no fork/cherry-pick I did not make).
  • No secrets / credentials / large binaries staged.

Summary

TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.

Test plan

  • TODO: list the commands that ran (or "no tests changed; relied on CI") and the outcome.

gunbc-ci-auto-heal and others added 11 commits August 23, 2026 23:48
…o fleet convergence

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ptiy92YHu7ig5W7ktiBzpZ
The annotation sat inside the service declaration body, which DESIGN 4c does not model.
Found by execution: claim_executor --required-ci parse phase, seven refusals on this file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ptiy92YHu7ig5W7ktiBzpZ
Two rows answered the same question and disagreed. Every BuildCacheInstance
carried gunbc_managed_compile_pool_slice as intended_compile_pool
unconditionally, while gunbc_compile_pool_placement said
CompilePoolInRunnerSlots -- so the rendered cache unit named a Slice= that
nothing provisions. Applying it would have had systemd create that cgroup
implicitly WITH NO LIMITS: the same unbounded compile charge
gunbc_compile_pool_doc measured, wearing the managed topology's name, with the
budget model believing a bounded pool existed. Worse than the placement it was
meant to improve on, because it looks converged.

THE ROW MOVES TO gunbc.host_layout, AND THE HOME WAS FORCED BY MEASUREMENT
RATHER THAN CHOSEN. Deriving intended_compile_pool from the placement row
where it stood is impossible: gunbc.fleet_host_budget transitively REACHES
gunbc.build_cache_instance, so the instance importing it closes a cycle, and
acyclicity is the one structural law the import graph has. host_layout reaches
neither consumer and both already import it, so the single authority costs no
new import edge. The semantic argument agrees with the measurement -- where
compile RSS is charged is a fact about how a host is laid out, which is that
module's whole subject, and it already owns the slice NAME. The name and
whether the name is managed were always one question in two homes.

THE FIX IS CONSTRUCTION, NOT A CHECK. intended_compile_pool stops being a bare
NonEmptyStr and carries CompilePoolPlacement itself, sourced from the single
row. build_cache_unit then renders Slice= through a match, so the
CompilePoolInRunnerSlots arm has NO SLICE NAME TO RENDER and the hazardous unit
is not constructible -- rather than being constructible and avoided by
remembering. runner_activation's pool-receipt binding answers false on that arm
instead of comparing a receipt against a pool this fleet does not declare.

THE ROW IS NOT FLIPPED. The fleet still declares CompilePoolInRunnerSlots, so
both live tripwires keyed on that value stay green:
test.claim.compile_pool_ensure_wiring_witness
witness_live_topology_refuses_the_slice_ensure and
test.claim.host_compile_pool live_topology_doc. The flip belongs with the wet
convergence that installs the slice, which is what the dissolution trigger
already demands.

NOT VERIFIED LOCALLY: a whole-tree compile is OOM-killed in a session
container (REAL_EXIT=137, swap disabled, shared slice), so this relies on CI
to compile it. Reviewers should treat the required run as the first real
check, not a formality.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ServiceDirective is a nickname for SystemdServiceDirective, which is what
extdeps.systemd.unit_file declares and what every other directive in this
render already uses. Not a missing import: the bare name resolves nowhere
in the corpus.

Found by execution: required-ci floor, strict preparation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ptiy92YHu7ig5W7ktiBzpZ
…y staging authority

Two review findings (codex/gpt-5.6-sol, review 55280), both correct.

1. Every other HostToolchainKind is reached through a named func; CompilePool
   had none, so the kind was reachable from the dispatcher and the dispatcher
   from nobody. gunbc.host_compile_pool_provision provision_compile_pool is
   that entry point, thin, carrying no policy.

2. The install body hand-assembled a cat-heredoc and a sudo -n install --
   a second authority for staging-and-installing a unit file beside the one
   gunbc.live_deploy.operations already owns. It now routes through
   deploy_stage_write_command and deploy_stage_install_command, which build
   from bash_build nodes rather than string assembly. The residue is the two
   lines live_deploy itself declares unmodelled (the stage-dir Let and the
   trap), both derived from deploy_stage_dir_var.

Three witnesses added, each with an authorable RED: the first draft of the
install body contained exactly the heredoc and sudo-install they refuse.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ptiy92YHu7ig5W7ktiBzpZ
An import list does not bind names here, but it does decide which modules a
run LOADS, and gunbc.host_compile_pool_provision is a leaf entry point that
nothing imports. Its symbols were absent rather than unbound, so the
entry-point witness failed NoSuchFunction under gunbc run while passing under
the floor, which loads all 3873 modules regardless.

Measured: all eight witnesses now resolve, typecheck and evaluate to true.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ptiy92YHu7ig5W7ktiBzpZ
…on instance

The placement carrier made intended_compile_pool conditional, and three floor
witnesses went red. None of them was testing what its name says.

- build_cache_ensure witness_the_generated_unit_reproduces_the_captured_environment
  asserted Slice=gunbc-compile-pool.slice as part of a CAPTURED environment. That
  capture was an env var set, and its SCCACHE_STARTUP_NOTIFY entry is the tell that
  no unit existed on that host at all -- so the Slice= line was never captured, it
  was our own intent pinned inside a fidelity witness. Moved to a both-arms witness
  beside the capacity idiom the same file already uses: absent under InRunnerSlots,
  present under SliceDeclared.

- runner_activation_wall and runner_host_deploy built a fully-ready host from the
  PRODUCTION ci_cache_instance, so once the live row became the carrier they were
  asserting a fact about this fleet topology rather than about the activation fold.
  Fixtures now declare the pool; the live answer gets its own witness that asserts
  the fleet-wide refusal deliberately.

That new witness returned false on its first run -- fully_verified_ready derives its
instance from the fixture two calls away, so the live witness silently received the
pool-declared one and asserted a refusal that could not happen. Split into
fully_verified_ready_for so a witness chooses its own subject. Caught by running it.

Measured: all five resolve and return true, including the negative control
witness_bare_host_admission_cannot_yield_an_enable_command.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ptiy92YHu7ig5W7ktiBzpZ
* Produce BuildCacheInstanceReady from exact observations

* Bind unavailable cache observations to endpoint owner

* Refuse foreign unavailable observations before classification

---------

Co-authored-by: Brian Searls <briansearls1@gmail.com>
@gunbai-bot

gunbai-bot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Closing without merging. Adopted for review after this PR's session ended; the investigation is recorded here so the next reader finds it rather than redoing it.

Nothing here is a criticism of the work. The refactor underneath is real and worth finishing — see What is worth salvaging at the bottom.

1. This branch's earlier work already merged, as #9062

#9062 — same title, same branch session/stern-boar-129 — merged 2026-08-24T21:40:12Z as bc992dbe6fc. This PR's head is 21:40:15Z, three seconds later. So this is not 23 files of unlanded work; it is the same branch's continuation, opened over a trunk that had just absorbed it.

That is why it advertises +1855. Squash-merge flattens the branch onto main without making its commits ancestors, so the merge base never advanced and a three-dot diff re-displays landed work as new:

three-dot (what this PR shows)        23 files, +1855/-94   ← misleading
two-dot, scoped to those same files    4 files,  +629/-173   ← what really differs from main

19 of the 23 files are byte-identical to main. The add/add conflict on the witness file is this branch colliding with its own already-merged copy — no third party's authority is involved.

2. Two latent reverts, and neither is visible in the conflict list

The branch predates three main repairs. A branch predating N repairs carries N latent reverts, and only enumeration finds them:

3. The residue does not build, and its own witnesses disagree

Reconstructed by branching from main and applying only the post-#9062 WIP:

Compile — clean main's host_effect_realize.dag as entry: 2 hard diagnostics (pre-existing, see below). With the WIP: 6. The 4 it introduces are shape errors that never compiled on either tree — destructuring TypedArgvExecConverged { exit_code, stdout, stderr } when that variant carries one field (result: SshSessionExecResult), and using Absent as a constructor where the Optional constructor is none. Both are mechanically fixable, and with them fixed the count returns to main's 2.

Witnesses: 14 of 16 pass. The two failures are the WIP's own new witnesses and are not mechanical:

  • witness_publish_is_typed_words_and_never_a_program is stale and contradicts its own siblings. It asserts publish installs directly to /etc/systemd/system/<slice> from /tmp/. Two siblings that pass assert the opposite on both counts — publication must not contain the destination, must write <dest>.gunbc-incoming with a separate mv -f renaming it, and the incoming name must not contain /tmp. One file, two eras, one function.
  • witness_unanchored_transport_refuses_instead_of_installing asserts the right thing while the code fails open. It expects SshShell to refuse with FleetSshExecutionContext / "No unit written"; the implementation routes SshShell straight to realize_ensure_compile_pool_slice_body and installs.

4. Why this closes rather than lands trimmed

Landing the 14 green and dropping the 2 reds would ship a publication path that installs over SSH without the anchoring its own sibling witness demands — behind a green. That is not an incomplete feature; it is a fail-open with a passing suite in front of it. And deleting the stale witness to reach green is the §5 antipattern verbatim: satisfying a check by editing the declaration while the realization goes unexamined.

What is worth salvaging

The typed-publication refactor is genuine: host_effect_realize.dag stops forking the deploy staging authority and routes publication through gunbc.typed_remote_file_write (converge_typed_remote_file, typed_remote_file_write_seal), with witnesses covering typed publication, rename-not-truncate, incoming-as-sibling, declared mode authority, and seal scope. It also makes systemctl_start_command dead — on main that symbol has exactly 3 occurrences (one declaration in live_deploy/operations.dag, two uses in host_effect_realize.dag), all removed by the refactor.

It should be re-filed as one scoped piece of work — the typed-publication refactor plus the SshShell → FleetSshExecutionContext anchoring, which is migration surface across 25 files under dag/ + src/v2/, not re-cut surface. The two are inseparable: the refactor's own witness already demands the anchoring.

Unrelated, found in passing

Clean main carries 2 blocking diagnostics in host_effect_realize.dag today — no field 'Cli' on type 'gunbc' and method 'Run' cannot be resolved. Not this PR's. Diagnosed with a positive control: gunbc.Cli is declared (gunbc/cli_services.dag), and the failing modules simply do not import gunbc.cli_services — typed_witness_invocation_test.dag calls gunbc.Cli.Run, imports it, and compiles clean, while host_effect_realize.dag and host_codex_runtime_provision.dag call it without the import and both report the pair. Routed separately for triage; not fixed from here.

@gunbai-bot gunbai-bot Bot closed this Aug 25, 2026
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.

0 participants