Skip to content

Stop the installer witness rendering a refusal as an empty string - #9038

Closed
gunbai-bot[bot] wants to merge 4 commits into
mainfrom
session/bold-raven-901
Closed

gunbai-bot[bot] wants to merge 4 commits into
mainfrom
session/bold-raven-901

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

Reduced by ruling to the half nothing else carries: one conflation repair in srv4_runner_installer_command_cites_installer_with_env.

What this fixes

The witness rendered a refusal as an empty string and then pattern-matched it:

let c = match outcome {
  RunnerCommandReady { command: cmd } => shell_command_render(command: cmd)
  RunnerCommandRefused { host_label: _, reason: _ } => ""
}
string_contains(s: c, pattern: "'CTRL_RUNNER_HOST_LABEL=srv4'") && ...

An empty render fails every string_contains below it, so "the admission refused" and "the command was built and says something else" produce the same false — two facts with opposite repairs (one sends you to the admission chain, the other to the renderer) collapsed into one uninformative bit. That is the not-applicable-versus-malformed conflation in DESIGN's recurring failure modes.

The assertions now run inside the RunnerCommandReady arm, where a command actually exists; the refusal arm returns false on its own.

Honest boundary: a witness returns Bool, so the arm still cannot carry the NonEmptyStr reason out to the log. What the split buys is that a refusal can no longer masquerade as four simultaneously-false string conjuncts. Surfacing the reason itself needs a richer witness carrier and is deliberately not attempted here.

What was dropped, and why

This PR originally also repinned the elevation words in srv4_enables_its_declared_runner_instances. #9031 (fix/argv-command-ls-seal) already carries a fix for that row, authored directly on that branch minutes before this PR opened — neither side could see the other. That row is restored here to main's text verbatim, so the two edits cannot collide and #9031's version lands.

#9031's version is also the better one, and my stated reason for keeping a literal was wrong. I invoked DESIGN's oracle rule (automating a literal's update must not collapse the assertion to measure() == measure()). The rule is right; its premise is absent, because there are two producers: the pattern would derive from extdeps.sudo.elevation / extdeps.systemd.systemctl, while the searched string is produced by runner_activate_command's builder. measure(builder) == measure(authority) is exactly the join worth asserting — that the builder routes through the authorities rather than spelling its own words — and it stays discriminating: hardcode a bare sudo in the builder against an authority saying /usr/bin/sudo and the derived conjunct goes red.

My literal also had the failure mode I was trying to avoid, pointing the other way: a pinned 'sudo' 'systemctl' 'enable' '--now' is a third authoring of facts two extdeps modules already own, and it rotted the moment activation routed through systemctl_enable_now_command — which is why that row was red. #9031 derives what an authority owns and leaves enable / --now literal, because those are systemctl's own operands the builder spells inline and no row owns.

Class census — CORRECTED to 6 (was 9)

The 9-site table previously here was wrong: my scanner counted braces without skipping string literals, so a pattern like "\"activity\": {" unbalanced the counter and attributed later hits to the wrong function. Three sites (roadmap_belt_actuate, roadmap_provider_events, runner_placement) do not contain the arm at all. Re-run with controls both ways — the known-true site is found, the site fixed in this PR is correctly absent.

Real count under the same predicate (a match arm returning "" inside a test fn whose body calls string_contains): 6.

site disposition
runner_host_deploy srv4_enables_… owned by #9031
long/roadmap_page × 2 (EmitRejected) genuine — being fixed in the lane
typed_remote_file_converge × 2 left alone: inverted polarity, "" is the success arm and each row guards it
build_cache_endpoint_observe (Absent) left alone: an exact-equality conjunct makes the refusal case genuinely red

Separately: long/roadmap_page has 11 EmitRejected => "" sites, 8 of them in shared render helpers that absorb the rejection before any witness sees it. That form needs helper signature changes and is referred for a decision, not improvised.

CI

The witnesses red on this branch is main's pre-existing ArgvCommand sole_constructor break at dag/gunbc/runner_slot_provision.dag:240 (from #8992), which refuses at strict-preparation before any witness executes. Not from this branch — which touches one file — and not closable from it. #9031 carries that seal fix; re-run this after it lands.

🤖 Generated with Claude Code

https://claude.ai/code/session_014oG2tMMAxgZeAQxDjvNsFm

… its refusal arm from erasing the cause

srv4_enables_its_declared_runner_instances was red on main, and not on the
roster/units join it exists to check. Its last conjunct pinned
'sudo' 'systemctl' 'enable' '--now', which was the rendering before
extdeps.sudo.elevation grounded the elevation words -- sudo_binary_path at the
absolute /usr/bin/sudo, sudo_non_interactive_flag at -n -- and
systemctl_enable_now_command now builds its argv from those rows. Both modules'
own annotations record that re-spelling as deliberate ("callers that previously
spelled a bare sudo now render this row -- a real change in the emitted words"),
so the witness was the straggler that update missed, not a defect in the fleet
model. The pattern is repinned as a literal rather than rebuilt from those two
rows, because deriving it from the same authority the builder consumes would
make the conjunct agree with itself whatever the rows say.

The second half of the brief is why neither falsification of this row located
itself. The refusal arm collapsed to "", so an activation refusal and a command
that says something else both arrived as one indistinguishable false -- every
string_contains below fails identically on an empty render. That is the
not-applicable-versus-malformed conflation DESIGN names, sitting in a witness.
Refusal is now its own match arm returning false directly, and the content
assertions run only inside RunnerCommandReady, where a command actually exists.
The same collapse in srv4_runner_installer_command_cites_installer_with_env is
fixed the same way, since it was one shape written twice.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014oG2tMMAxgZeAQxDjvNsFm
@gunbai-bot

gunbai-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

The witnesses red is not from this PR and cannot be closed from this branch.

required-ci: floor refused: subject=82591b9b97623526 modules_resolved=3847 modules_excluded=4
dag/gunbc/runner_slot_provision.dag:240:14: error: sole_constructor type 'ArgvCommand'
    cannot be constructed outside its defining module

Three facts:

  1. This branch touches exactly one file — git diff --name-only origin/main...HEAD returns only dag/test/claim/runner_host_deploy_witness_test.dag.
  2. The offending construction is on main verbatim (ArgvCommand { argv: [...] } at runner_slot_provision.dag:240, from Runner slot membership becomes fleet-converge's third member family, plan and apply #8992). Nothing here modified it.
  3. It refuses at strict-preparation, before the fold — so zero witnesses executed in this run. That is the same compile-time masking that hid srv4_enables_its_declared_runner_instances on main to begin with: the row this PR fixes could not have run in this job in either direction.

gunbc#9031 carries the seal fix. Once it lands, this branch should be re-run rather than repaired.

The fix in this PR is green by execution independently — remote single-dispatch claim_batch, whole-corpus resolve of the witness entry (378 modules in closure):

PASS srv4_enables_its_declared_runner_instances
PASS srv4_runner_installer_command_cites_installer_with_env
PASS enrolled_host_still_admits

with the paired RED being main's current state of the same row.

— sent from bold-raven-901

…ion ruling is right

RULING ACCEPTED (swift-badger-524). #9031 fix/argv-command-ls-seal already
carries a fix for srv4_enables_its_declared_runner_instances, authored directly
on that branch minutes before this PR opened, and it is the branch unblocking
main's ledger. That row is restored here to main's text verbatim so the two
edits cannot collide; #9031's version lands.

I WAS WRONG ABOUT THE DERIVATION AND THE REASON MATTERS. I kept the elevation
words as a literal and cited DESIGN's oracle rule -- automating a literal's
update must not collapse the assertion to measure() == measure(). That rule is
correct and its premise is absent here, because there are TWO producers, not
one: the witness's pattern would derive from extdeps.sudo.elevation and
extdeps.systemd.systemctl, while the string being searched is produced by
runner_activate_command's BUILDER. measure(builder) == measure(authority) is
the join actually worth asserting -- that the builder routes through the
authorities instead of spelling its own words -- and it stays discriminating:
hardcode a bare sudo in the builder while the authority says /usr/bin/sudo and
the derived conjunct goes red. The collapse I feared needs the pattern derived
from the builder's own output, which nobody proposed.

Worse, my literal had the failure mode I was avoiding, pointing the other way: a
pinned 'sudo' 'systemctl' 'enable' '--now' is a THIRD authoring of facts two
extdeps modules already own, and it rotted the moment activation routed through
systemctl_enable_now_command -- which is why the row was red at all. Repinning
it buys one green run and re-arms the trap. #9031 derives what an authority owns
and leaves 'enable' and '--now' literal, because those are systemctl's own
operands that the builder spells inline and no row owns. That is the general
line and it is better than what I wrote.

WHAT REMAINS IS THE HALF NOTHING ELSE CARRIES. The installer witness rendered
RunnerCommandRefused as "" and then pattern-matched the empty string, so a
REFUSAL and a WRONG RENDER produced the same false with opposite repairs -- the
not-applicable-versus-malformed conflation. The assertions now run inside the
Ready arm and the refusal arm returns false on its own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014oG2tMMAxgZeAQxDjvNsFm
@gunbai-bot gunbai-bot Bot changed the title Repin the srv4 enable witness on the grounded sudo spelling, and stop its refusal arm from erasing the cause Stop the installer witness rendering a refusal as an empty string Aug 23, 2026
@gunbai-bot

gunbai-bot Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

Closing: this PR's content is already on main, landed through #9025.

Verified rather than assumed — 3009a5f4e56 ("Fix record-like if condition brace parsing", #9025) carries an identical change to dag/test/claim/runner_host_deploy_witness_test.dag: same file, same stat (9 insertions, 7 deletions), same hunk:

-    RunnerCommandRefused { host_label: _, reason: _ } => ""
+    RunnerCommandRefused { host_label: _, reason: _ } => false

And main's srv4_runner_installer_command_cites_installer_with_env is now byte-identical to this branch's version. So merging this would be a no-op, and leaving it open costs the operator a slot in a queue being worked by hand.

Closing rather than merging is the honest disposition: the repair is delivered, just not by this PR. The two approvals it collected (reviews 55115, 55169) attach to a change that is now on main by another route, so nothing is lost.

What is not on main, and where it went: the same conflation in srv4_enables_its_declared_runner_instances — main still has RunnerCommandRefused { host_label: _, reason: _ } => "" there, so a refusal and a wrong command still arrive as one false on that row. That is the half this PR never carried (I restored that row to main's text verbatim so it could not collide with #9031's derived arm-6 repair, which has since landed). It is now a separate PR against current main.

— sent from bold-raven-901

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.

1 participant