From 09623031d3d147ca81d2a0523472ffce78661b67 Mon Sep 17 00:00:00 2001 From: gunbc-ci-auto-heal Date: Thu, 20 Aug 2026 03:20:00 +0000 Subject: [PATCH 1/2] command_runner: collapse the transport match at the run site onto command_over_transport command_over_transport already resolves both CommandTransport arms into one plain, directly-executable outer ArgvCommand (LocalExec is the identity; SshExec folds the inner command into one RFC 4254 command string, landed in #8596). Re-deciding the transport at command_runner's run site duplicated that dispatch at its consumer. run_shell_command and run_shell_command_capture now ask command_over_transport once, decompose the resulting argv into program + operands, and hand both straight to shell.Exec.RunArgv -- no bash heredoc, no shell re-parse, no per-transport match, for either transport. Empty argv refuses rather than defaulting. Adds cli_surface_of_literal_words, the one sanctioned route from a runtime List to a CliSurface, and a wet discriminating receipt proving the words reach the process unsplit (excluded from the hermetic floor since the property isn't observable without a real process). Corrects two now-stale claims about command_runner's heredoc path in bmc_netboot_serve.dag and the shell-to-dag residual census doc. Co-Authored-By: Claude Sonnet 5 --- dag/gunbc/bmc_netboot_serve.dag | 2 +- dag/gunbc/command_runner.dag | 101 ++++++++++++++---- ...command_runner_local_argv_receipt_test.dag | 66 ++++++++++++ ...-dag-residual-census-and-arc-completion.md | 4 +- src/v1/stage0/src/cli_run.rs | 7 ++ src/v2/std/compilers/cli_surface.dag | 28 +++++ 6 files changed, 182 insertions(+), 26 deletions(-) create mode 100644 dag/test/manual/command_runner_local_argv_receipt_test.dag diff --git a/dag/gunbc/bmc_netboot_serve.dag b/dag/gunbc/bmc_netboot_serve.dag index 6e7d95a4b3a..ac27b2d5a58 100644 --- a/dag/gunbc/bmc_netboot_serve.dag +++ b/dag/gunbc/bmc_netboot_serve.dag @@ -158,7 +158,7 @@ data bmc_netboot_provision_scaffold: Disposition = Scaffold { } } -data bmc_netboot_provision_dissolution_trigger: DissolutionCondition = unbound_dissolution(description: "๐ŸŸก dissolve-on: bmc_netboot_provision โ€” the staging manifest (bmc_netboot_staging_manifest) IS the driver: provision folds its BmcStagedFile rows through bmc_netboot_stage_file (LocalArtifact scp / InlineText write+scp / FetchFromUri typed refusal), so the file list has one authority and the former hand-unrolled sequence is deleted. bmc_netboot_run_bmc and bmc_netboot_put_path no longer hand-roll their own retained_runtime/shell.Exec.Run heredoc glue (that was two bridge sites duplicating one pattern, ยง2/ยง3); both now call gunbc.command_runner.run_shell_command, the single authority for rendering an ArgvCommand+CommandTransport over the heredoc bridge โ€” command_runner already carries its own dissolution obligation (command_runner_dissolution_trigger) for that bridge, so this module inherits rather than duplicates it. CORRECTED 2026-08-19 (this clause previously said the SshExec debt below was unchanged and out of scope; that is now false and is rewritten rather than left standing per DESIGN ยง3's stale-citation discipline): extdeps.exec.command command_over_transport's SshExec branch no longer composes ssh_exec_prefix ++ argv as a flat suffix. It folds the inner ArgvCommand through gunbc.remote_shell_command remote_exec_command_string into ONE RFC-4254-conformant, POSIX-single-quote-encoded command string (cited to IEEE 1003.1-2017 section 2.2.2), carried as ssh's single final argument โ€” bmc_transport now names openbmc_dropbear_interpretation explicitly on SshExec so this fold runs, rather than refusing on Unknown. Remaining debt is scoped narrower: the heredoc bridge itself (command_runner's shell_exec_via_bash/retained_runtime hop) is unchanged. DISSOLVES WHEN host_effect_apply binds ArgvCommand execution (mkdir / scp / busybox-httpd-launch) as typed staging effects, and dag/gunbc/bmc_netboot_serve.dag retires command_runner.run_shell_command in favor of host_effect_apply") +data bmc_netboot_provision_dissolution_trigger: DissolutionCondition = unbound_dissolution(description: "๐ŸŸก dissolve-on: bmc_netboot_provision โ€” the staging manifest (bmc_netboot_staging_manifest) IS the driver: provision folds its BmcStagedFile rows through bmc_netboot_stage_file (LocalArtifact scp / InlineText write+scp / FetchFromUri typed refusal), so the file list has one authority and the former hand-unrolled sequence is deleted. bmc_netboot_run_bmc and bmc_netboot_put_path no longer hand-roll their own retained_runtime/shell.Exec.Run heredoc glue (that was two bridge sites duplicating one pattern, ยง2/ยง3); both now call gunbc.command_runner.run_shell_command, the single authority for rendering an ArgvCommand+CommandTransport over the heredoc bridge โ€” command_runner already carries its own dissolution obligation (command_runner_dissolution_trigger) for that bridge, so this module inherits rather than duplicates it. CORRECTED 2026-08-19 (this clause previously said the SshExec debt below was unchanged and out of scope; that is now false and is rewritten rather than left standing per DESIGN ยง3's stale-citation discipline): extdeps.exec.command command_over_transport's SshExec branch no longer composes ssh_exec_prefix ++ argv as a flat suffix. It folds the inner ArgvCommand through gunbc.remote_shell_command remote_exec_command_string into ONE RFC-4254-conformant, POSIX-single-quote-encoded command string (cited to IEEE 1003.1-2017 section 2.2.2), carried as ssh's single final argument โ€” bmc_transport now names openbmc_dropbear_interpretation explicitly on SshExec so this fold runs, rather than refusing on Unknown. CORRECTED AGAIN (2026-08-20, this clause previously said the heredoc bridge -- command_runner's shell_exec_via_bash/retained_runtime hop -- was unchanged; that is now false too and is rewritten rather than left standing, same ยง3 discipline): command_runner's run site no longer matches on transport or hops through a bash heredoc for either arm -- it asks command_over_transport once for the outer ArgvCommand and hands it straight to shell.Exec.RunArgv. So this module's remaining debt is scoped to command_runner_dissolution_trigger's own first clause (host_effect_apply has not yet bound ArgvCommand execution), which command_runner still carries and this module inherits rather than duplicates. DISSOLVES WHEN host_effect_apply binds ArgvCommand execution (mkdir / scp / busybox-httpd-launch) as typed staging effects, and dag/gunbc/bmc_netboot_serve.dag retires command_runner.run_shell_command in favor of host_effect_apply") func bmc_netboot_run_bmc(plan: BmcNetbootServePlan, argv: List) -> ProcessExit uses net: Network diff --git a/dag/gunbc/command_runner.dag b/dag/gunbc/command_runner.dag index 248b038649c..6d4ce3be0af 100644 --- a/dag/gunbc/command_runner.dag +++ b/dag/gunbc/command_runner.dag @@ -5,6 +5,10 @@ import std.dissolution { DissolutionCondition, dissolution_description, unbound_ import std.types { List, String, NonEmptyStr } import std.process { ProcessExit, ExitSuccess, exit_failure } import std.resources { Network } +import v2.std.algebra { list_head, list_tail, HeadFound, HeadAbsent, TailFound, TailAbsent } +import std.algebra { Empty } +import v2.std.compilers.cli_surface { cli_surface_of_literal_words, process_argv_expansion } +import extdeps.shell.exec { ExitZeroSucceeds, process_exit_is_admitted } import extdeps.exec.command { ArgvCommand, CommandTransport, @@ -12,7 +16,6 @@ import extdeps.exec.command { command_over_transport, CommandOverTransportBuilt, CommandOverTransportRefused, - shell_command_render, } import gunbc.remote_shell_command { RemoteExecCommandRefusalCause, @@ -20,8 +23,6 @@ import gunbc.remote_shell_command { SimpleCommandArgvRefused, } import extdeps.shell -import gunbc.shell_bash_runner { shell_exec_via_bash } -import gunbc.retained_shell_script { retained_runtime } import std.disposition { Disposition, Scaffold, RealizationDispatch } import std.decl_ref { DeclarationRef, WholeDeclaration } @@ -34,8 +35,29 @@ data command_runner_scaffold: Disposition = Scaffold { } } -data command_runner_dissolution_trigger: DissolutionCondition = unbound_dissolution(description: "๐ŸŸก dissolve-on: run_shell_command โ€” generic ArgvCommand runner routes the typed extdeps.exec.command shape through shell.Exec.Run via the shell_bash_runner heredoc transport; DISSOLVES WHEN host_effect_apply binds ArgvCommand execution as a typed transport handler and dag/gunbc/command_runner.dag retires its shell_exec_via_bash glue") +data command_runner_dissolution_trigger: DissolutionCondition = unbound_dissolution(description: "๐ŸŸก dissolve-on: run_shell_command โ€” generic ArgvCommand runner routes the typed extdeps.exec.command shape directly at shell.Exec.RunArgv, one uniform arm for every CommandTransport rather than a per-transport bash heredoc; DISSOLVES WHEN host_effect_apply binds ArgvCommand execution as a typed transport handler and dag/gunbc/command_runner.dag retires run_shell_command in favor of it") +// THE RUN SITE NO LONGER MATCHES ON TRANSPORT. +// +// command_over_transport is ALREADY transport-agnostic at its output: LocalExec is the identity +// and SshExec folds the inner command into ONE RFC-4254 command string carried as the final word +// of an outer sshpass/ssh argv (extdeps.exec.command command_over_transport_composition_note). +// Either way the result is one plain ArgvCommand of local-machine words -- there is no longer a +// reason to ask which transport built it. A per-transport match here would be re-deciding a +// question command_over_transport already answered (DESIGN ยง3: dispatch belongs at one seam, not +// duplicated at its consumer). +// +// So this runner has exactly one execution path: decompose the outer argv into program + operand +// words and hand them straight to shell.Exec.RunArgv, with no shell, no quoting, and no re-parse +// on this machine -- for LocalExec the words go directly to the named binary; for SshExec the +// words go directly to sshpass/ssh, whose own single final argument already carries the correctly +// composed and quoted remote command string. Nothing about that composition depends on running it +// through a local shell first. +// +// EMPTY ARGV REFUSES rather than defaulting. A command with no words names no program to execute, +// and inventing one would be the fabricated output DESIGN ยง5 forbids. command_over_transport never +// produces an empty argv on either arm (SshExec always prepends sshpass/ssh/target/opts), so this +// arm is reachable only from a caller-constructed empty ArgvCommand on LocalExec. func run_shell_command(command: ArgvCommand, transport: CommandTransport) -> ProcessExit uses net: Network { @@ -43,21 +65,35 @@ func run_shell_command(command: ArgvCommand, transport: CommandTransport) -> Pro CommandOverTransportRefused { cause: c } => exit_failure(reason: concat("command_over_transport refused: ", command_over_transport_refusal_reason(cause: c))) CommandOverTransportBuilt { command: over } => - let rendered = shell_command_render(command: over) - let run = shell.Exec.Run( - script: retained_runtime( - body: shell_exec_via_bash(body: rendered), - reason: "generic ArgvCommand runner heredoc transport, pending the typed host_effect_apply argv handler" as NonEmptyStr, - ) - ) - if run.success { - ExitSuccess - } else { - exit_failure(reason: concat("command failed: ", run.stderr)) + match list_head(xs: over.argv) { + HeadAbsent => + exit_failure(reason: "empty argv: a command with no words names no program to execute") + HeadFound { value: program } => { + let run = shell.Exec.RunArgv( + program: program as NonEmptyStr, + arguments: process_argv_expansion( + surface: cli_surface_of_literal_words(words: argv_operands(command: over)) + ), + ) + if process_exit_is_admitted(policy: ExitZeroSucceeds, exit_code: run.exit_code) { + ExitSuccess + } else { + exit_failure(reason: concat("command failed: ", run.stderr)) + } + } } } } +// The operands are everything after the program. A missing tail is the one-word command, whose +// operand list is empty -- not an error, and not the same state as an empty argv. +fn argv_operands(command: ArgvCommand) -> List { + match list_tail(xs: command.argv) { + TailAbsent => Empty + TailFound { tail: rest } => rest + } +} + data command_over_transport_refusal_reason_note: String = "Renders RemoteExecCommandRefusalCause (gunbc.remote_shell_command) into a diagnostic String for ProcessExit -- the only refusal this generic runner can hit today is RemoteExecInterpretationUnknown, since every SshExec constructed in-tree names a determined interpretation (openssh_login_shell_interpretation / openbmc_dropbear_interpretation); the cause stays a match over the full type rather than a partial one so a newly added cause fails to compile here instead of falling through silently (DESIGN 4b: no untracked stall)." fn command_over_transport_refusal_reason(cause: RemoteExecCommandRefusalCause) -> String { @@ -79,6 +115,12 @@ type ShellCaptureResult { stderr: String } +// THE success FIELD IS DERIVED THROUGH A NAMED POLICY, not a bare exit_code == 0. RunArgv +// deliberately carries no success field, because success is a POLICY judgement about an +// observation rather than part of it -- jq exits 4 to mean "no output" and 1 to mean "false +// result", neither of which is failure. Routing through process_exit_is_admitted(ExitZeroSucceeds) +// states which convention this runner applies, so a caller needing a different one has something +// to change rather than a literal to discover. func run_shell_command_capture(command: ArgvCommand, transport: CommandTransport) -> ShellCaptureResult uses net: Network { @@ -90,14 +132,27 @@ func run_shell_command_capture(command: ArgvCommand, transport: CommandTransport stderr: concat("command_over_transport refused: ", command_over_transport_refusal_reason(cause: c)), } CommandOverTransportBuilt { command: over } => - let rendered = shell_command_render(command: over) - let run = shell.Exec.Run( - script: retained_runtime( - body: shell_exec_via_bash(body: rendered), - reason: "generic ArgvCommand capture runner heredoc transport, pending the typed host_effect_apply argv handler" as NonEmptyStr, - ) - ) - ShellCaptureResult { success: run.success, stdout: run.stdout, stderr: run.stderr } + match list_head(xs: over.argv) { + HeadAbsent => + ShellCaptureResult { + success: false, + stdout: "", + stderr: "empty argv: a command with no words names no program to execute", + } + HeadFound { value: program } => { + let run = shell.Exec.RunArgv( + program: program as NonEmptyStr, + arguments: process_argv_expansion( + surface: cli_surface_of_literal_words(words: argv_operands(command: over)) + ), + ) + ShellCaptureResult { + success: process_exit_is_admitted(policy: ExitZeroSucceeds, exit_code: run.exit_code), + stdout: run.stdout, + stderr: run.stderr, + } + } + } } } diff --git a/dag/test/manual/command_runner_local_argv_receipt_test.dag b/dag/test/manual/command_runner_local_argv_receipt_test.dag new file mode 100644 index 00000000000..52c26d4eb8a --- /dev/null +++ b/dag/test/manual/command_runner_local_argv_receipt_test.dag @@ -0,0 +1,66 @@ +module test.manual.command_runner_local_argv_receipt + +import std.types { Bool, String } +import extdeps.exec.command { ArgvCommand, LocalExec } +import gunbc.command_runner { run_shell_command_capture } + +// THE DISCRIMINATING RECEIPT FOR command_runner's TRANSPORT-AGNOSTIC RUN SITE. +// +// WHAT IT DISCRIMINATES, which is the only thing that makes it worth running. The run site used +// to render an ArgvCommand to quoted text and hand it to bash, which split it back apart on +// whitespace -- and for a time carried a per-transport `match` at the call site that duplicated +// the dispatch command_over_transport already performs. Both are gone: run_shell_command_capture +// now asks command_over_transport ONCE for the outer ArgvCommand (LocalExec is the identity; +// SshExec already folds the remote program into one RFC 4254 string) and hands that argv, +// word-for-word, straight to shell.Exec.RunArgv -- no shell, no re-parse, no transport match. A +// test that merely runs a command green proves nothing about that change -- it would be equally +// green with the bash hop restored. So the subject is an argument the two paths DISAGREE about: +// one containing a space. +// +// argv: ["printf", "%s|", "a b"] +// direct exec: one operand "a b" -> stdout "a b|" +// via a shell: two operands "a" and "b" -> stdout "a|b|" +// +// If this ever reads "a|b|", the run site has reached a shell again and the collapse has silently +// regressed. That is the failure this file exists to make loud. +// +// WHY IT IS WET AND NOT ENROLLED. Hermetic evaluation replays an operation's declared +// mock_response off the declaration and never constructs an argv at all, so the property under +// test -- how the words reach the process -- is not observable hermetically by construction. A +// hermetic version of this file could only assert the mock, which is the specification-without- +// execution trap. It therefore runs wet, by hand, and is excluded from the floor exactly as +// test/manual/process_argv_expansion_receipt_test.dag is, for the same reason. + +test fn spaced_operand_survives_as_one_argument() -> Bool { + run_shell_command_capture( + command: ArgvCommand { argv: ["printf", "%s|", "a b"] }, + transport: LocalExec, + ).stdout == "a b|" +} + +// The RED half of the same observation, stated as its own assertion rather than left implicit in +// the equality above: the shell-split rendering must NOT be what comes back. Written separately so +// a future edit that weakens the positive into something like a nonempty check still trips here. +test fn shell_split_rendering_is_not_what_comes_back() -> Bool { + run_shell_command_capture( + command: ArgvCommand { argv: ["printf", "%s|", "a b"] }, + transport: LocalExec, + ).stdout != "a|b|" +} + +// Control that the runner works at all on the ordinary case, so a failure above is read as a +// splitting regression rather than as the whole path being broken. +test fn plain_operand_round_trips() -> Bool { + run_shell_command_capture( + command: ArgvCommand { argv: ["printf", "%s|", "plain"] }, + transport: LocalExec, + ).stdout == "plain|" +} + +// Empty argv refuses rather than defaulting -- a command with no words names no program. +test fn empty_argv_refuses() -> Bool { + !run_shell_command_capture( + command: ArgvCommand { argv: [] }, + transport: LocalExec, + ).success +} diff --git a/docs/plans/shell-to-dag-residual-census-and-arc-completion.md b/docs/plans/shell-to-dag-residual-census-and-arc-completion.md index 317369add7b..e12e851b947 100644 --- a/docs/plans/shell-to-dag-residual-census-and-arc-completion.md +++ b/docs/plans/shell-to-dag-residual-census-and-arc-completion.md @@ -133,7 +133,7 @@ Two cautions against over-reading it. The `dag/gunbc` argv residue is **172 line **Do not turn these counts into a work plan.** Under the replacement-migration rule the unit is a root consumer ร— subject population, and the census direction reliably overestimates: sizing by argv occurrence, then by executable head, then by tool, each produced an inflated number, because a tool vertical, its site cutovers and its `host_effect_apply` caller are commonly **one** vertical whose acceptance condition is the old site's deletion โ€” not three workstreams. The domain clusters this document already tracks in ยง5.D, each with its own un-defer trigger, are the better unit. -**This section is a boundary and scoping increment, NOT a migration closeout receipt.** Neither finish line below is met at the commit introducing this text: `HostEffect.ShellCommand` still exists and is still constructed (including `host_effect_plan`'s empty placeholder), `command_runner`'s argv โ†’ text โ†’ heredoc route is live, and `retained_runtime` / `retained_srvn` remain defined and populated. Stated because a section naming a finish line reads, at a glance, like a claim to have reached it. +**This section is a boundary and scoping increment, NOT a migration closeout receipt.** Neither finish line below is met at the commit introducing this text: `HostEffect.ShellCommand` still exists and is still constructed (including `host_effect_plan`'s empty placeholder), and `retained_runtime` / `retained_srvn` remain defined and populated (elsewhere in the tree โ€” `bmc_netboot_serve`, `host_effect_realize`). Stated because a section naming a finish line reads, at a glance, like a claim to have reached it. *(Corrected 2026-08-20: this previously also named `command_runner`'s argv โ†’ text โ†’ heredoc route as live. That route is now deleted โ€” `run_shell_command`/`run_shell_command_capture` ask `command_over_transport` once for the outer `ArgvCommand`, for either transport, and hand it directly to `shell.Exec.RunArgv`, with no per-transport match and no shell round trip. `command_runner_dissolution_trigger`'s first clause โ€” `host_effect_apply` binding `ArgvCommand` execution โ€” is unaffected and stays open.)* ### Two finish lines, named separately @@ -154,7 +154,7 @@ SHELL-DAG is this document's; CLI-AUTHORITY is `cli-invocation-emission-design.m `extdeps.exec.command` `command_over_transport` constructs `ArgvCommand { argv: append(ssh_exec_prefix(target: t), items: command.argv) }`. That is exactly the `append(ssh_prefix, inner.argv)` composition ยง0c forbids โ€” SSH is a nested target whose carrier is one RFC 4254 command string, not a prefix on an argument vector. It is production code beneath the generic runner, so every SSH-transported `ArgvCommand` in the tree flows through it, and it is the concrete mechanism by which argument boundaries are lost remotely. -It sits in the same file as `gunbc.command_runner`'s round trip โ€” `run_shell_command` takes a typed `ArgvCommand`, renders it to quoted text via `shell_command_render`, and posts it through `shell.Exec.Run`'s heredoc transport. That carrier already declares the repair in `command_runner_dissolution_trigger`: *dissolves when `host_effect_apply` binds `ArgvCommand` execution as a typed transport handler*. So the runner cut and the SSH target are one vertical, not two, and the debt was declared before it was scoped. +It sits in the same file as `gunbc.command_runner`. *(Corrected 2026-08-20: this paragraph previously described `run_shell_command` as rendering the outer `ArgvCommand` to quoted text via `shell_command_render` and posting it through `shell.Exec.Run`'s heredoc transport. That round trip is deleted โ€” `run_shell_command` now asks `command_over_transport` once, per transport, and hands the resulting outer argv directly to `shell.Exec.RunArgv`, with no shell and no per-transport match at the run site.)* `command_runner_dissolution_trigger` still declares the repair its first clause owns: *dissolves when `host_effect_apply` binds `ArgvCommand` execution as a typed transport handler*. So the runner cut and the SSH target were one vertical, not two, and the debt was declared before it was scoped โ€” the SSH target landed in #8596, and the runner's own heredoc hop is now gone for both arms; what remains open is `host_effect_apply` itself. ### What is owed, and by whom diff --git a/src/v1/stage0/src/cli_run.rs b/src/v1/stage0/src/cli_run.rs index eb3a67936bc..183af6ac296 100644 --- a/src/v1/stage0/src/cli_run.rs +++ b/src/v1/stage0/src/cli_run.rs @@ -38665,6 +38665,13 @@ pub fn floor_prepared_subject_exclusions() -> Vec { // re-enrolls when a wet lane exists again โ€” which required_floor.dag deliberately defers // until it can be "asked against a live consumer". "test/manual/process_argv_expansion_receipt_test.dag".to_string(), + // Wet receipt for command_runner's transport-agnostic run site (the LocalExec/SshExec + // match collapsed onto command_over_transport). Excluded for the same reason as the line + // above and not a new class: hermetic evaluation replays an operation's declared + // mock_response and never constructs an argv, so "the words reached the process unsplit" + // is not observable hermetically. Enrolling it would assert the mock rather than the + // behaviour -- specification without execution. + "test/manual/command_runner_local_argv_receipt_test.dag".to_string(), ] } diff --git a/src/v2/std/compilers/cli_surface.dag b/src/v2/std/compilers/cli_surface.dag index f06fa062dd7..0781c358281 100644 --- a/src/v2/std/compilers/cli_surface.dag +++ b/src/v2/std/compilers/cli_surface.dag @@ -153,6 +153,34 @@ fn cli_argument_text(argument: CliArgument) -> String { // host boundary already names. A shell-hosted transport, if one is ever modeled, is a DIFFERENT // realization binding, not the thing this carrier describes. +// LITERAL WORDS INTO A SURFACE, and this is the ONE route for arguments whose spelling is a +// runtime value rather than a declared token. +// +// WHY IT IS NOT THE AMBIGUITY THIS CARRIER EXISTS TO CLOSE. The refused state is a List +// arriving at an argv position with NO declared role, where "one word whose text is the +// concatenation" and "N separate words" are both well-formed readings of the same value and the +// realization has to guess. This constructor IS the declaration: its name says each element is +// exactly one argv word, so the caller has chosen the reading rather than leaving it to be +// inferred. It is the same resolution the interpreter's refusal diagnostic tells authors to +// reach for. +// +// WHAT IT DOES NOT LICENSE. It is not a general escape from structural modeling. An argument +// whose spelling is known when the program is written belongs in CliArgumentSyntax fragments, +// which carry token classes and bindings and can be checked; this route carries none of that and +// can check none of it. It exists because argv words like a filesystem path or a hostname are +// genuinely runtime strings, and no fragment vocabulary can express a value that does not exist +// until the program runs. Reaching for it when fragments would have served is modeling debt, not +// a shortcut. +fn cli_surface_of_literal_words(words: List) -> CliSurface { + CliSurface { + arguments: fold_list( + xs: words, + empty: Empty, + cons: fn(acc, word) { list_snoc_item(xs: acc, item: CliArgument { text: word }) } + ) + } +} + type ProcessArgvExpansion sole_constructor { surface: CliSurface } fn process_argv_expansion(surface: CliSurface) -> ProcessArgvExpansion { From f4e02d4ffe08f37e3ebd90dcd06d5034a4bc96f8 Mon Sep 17 00:00:00 2001 From: gunbc-ci-auto-heal Date: Thu, 20 Aug 2026 04:17:07 +0000 Subject: [PATCH 2/2] command_runner: file the ssh exit-255 conflation as its own typed DissolutionCondition Records the four vivid-bee-590 design points durably in the tree rather than only in review discussion: the Built/Refused distinction and the LocalExec redesign latitude were already covered in prior comment blocks; this adds a dedicated ssh_exit_255_conflation_dissolution row (rung: mitigatable, next- rung trigger: an SshExec-typed 255 ProcessOutcome variant, gated on a real retry consumer) alongside the existing command_runner_dissolution_trigger, and trims the run-site comment to point at it instead of duplicating the same facts as prose the compiler cannot read as evidence. Co-Authored-By: Claude Sonnet 5 --- dag/gunbc/command_runner.dag | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/dag/gunbc/command_runner.dag b/dag/gunbc/command_runner.dag index e45e56deb2e..a59706ceb7d 100644 --- a/dag/gunbc/command_runner.dag +++ b/dag/gunbc/command_runner.dag @@ -41,6 +41,19 @@ data command_runner_scaffold: Disposition = Scaffold { data command_runner_dissolution_trigger: DissolutionCondition = unbound_dissolution(description: "๐ŸŸก dissolve-on: run_shell_command / run_shell_command_capture -- BOTH ARMS ARE NOW DONE, not just the local one. CORRECTED (this row previously said only the local arm routed straight at shell.Exec.RunArgv and that the SSH arm's nested command-string target was owned by another lane; that target landed in extdeps.exec.command command_over_transport (RFC 4254 fold, cited IEEE 1003.1-2017 section 2.2.2), which makes SshExec produce a plain, directly-executable outer ArgvCommand exactly as LocalExec's identity arm always did). With both arms yielding one plain ArgvCommand, re-deciding transport at this run site was re-answering a question command_over_transport already answered (DESIGN section 3: dispatch belongs at one seam). The per-transport match is deleted; both run_shell_command and run_shell_command_capture ask command_over_transport once, then hand the resulting argv straight to shell.Exec.RunArgv -- no TransportScript, no shell, no shell_command_render/shell_exec_via_bash/retained_runtime hop, for either transport. Measured before rewriting: command_runner remains the ONLY module that turns an ArgvCommand into a local process; the other ArgvCommand consumers reach shell_command_render for TEXT emission (githooks), a different destination. DISSOLVES WHEN host_effect_apply binds ArgvCommand execution as a typed transport handler and this module retires run_shell_command / run_shell_command_capture in its favor -- host_effect_apply exists (gunbc.host_effect_realize) but reaches zero ArgvCommand occurrences today, so that binding is still unbuilt.") +// RUNG: mitigatable. ssh overloads exit code 255 for its own transport failures (unreachable +// host, auth failure, connection reset), which classify_process_observation (extdeps.shell.exec) +// cannot distinguish by exit code alone from a remote command that legitimately exits 255 on its +// own terms -- both fold into one ProcessRefused { exit_code: 255, .. }. This collapse makes that +// the direct, un-mediated SshExec path (no intervening bash heredoc hop remains to obscure it); +// it does not introduce the collision, which was already latent in classify_process_observation. +// Harm is contained -- ProcessRefused is a typed, located refusal either way, never a silent wrong +// answer -- so the class does not fall below the floor; it simply cannot yet tell the two causes +// apart. ACCEPTED, NOT FIXED: disambiguating via ssh's stderr text is rejected, not deferred -- +// stderr is prose from an external process, and matching on it is exactly the heuristic DESIGN +// section 4/5 rules out where a richer source exists in principle but is unbuilt. +data ssh_exit_255_conflation_dissolution: DissolutionCondition = unbound_dissolution(description: "๐ŸŸก dissolve-on: run_shell_command_capture over SshExec -- transport is KNOWN at command_over_transport's call site, so an SshExec-transported ProcessRefused { exit_code: 255 } is distinguishable IN PRINCIPLE from a LocalExec one, and wants its own typed ProcessOutcome variant (a policy parallel to ExitZeroSucceeds) rather than sharing ProcessRefused with every other nonzero exit. The richer source is in-band remote status -- the remote side reporting its own exit through stdout instead of this caller trusting ssh's own process exit code -- not stderr-sniffing, which stays rejected as a heuristic. NOT BUILT HERE: this row names the trigger, it does not build the variant. DISSOLVES WHEN a real caller of run_shell_command_capture retries on SshExec -- retrying an unreachable host and retrying a command that actually ran and exited 255 are opposite, non-idempotent-risk-bearing decisions, so building the structural arm before a consumer exists to prove the distinction against would be speculative modeling, not grounded modeling (DESIGN section 4: a heuristic is never necessary in a closed system, but neither is a structural arm with nothing to hold it accountable).") + // WHAT GUARDS THIS, AND WHAT DOES NOT -- read before assuming the property below is enforced. // // The property "the local path reaches no shell" is established by execution, wet, against a real @@ -94,6 +107,18 @@ fn command_over_transport_refusal_reason(cause: RemoteExecCommandRefusalCause) - // command_over_transport never produces an empty argv on either arm (SshExec always prepends // sshpass/ssh/target/opts), so this arm is reachable only from a caller-constructed empty // ArgvCommand on LocalExec. +// +// THE BUILT/REFUSED DISTINCTION SURVIVES THE COLLAPSE, on purpose, as its own match arm rather +// than being folded into the exit-code decision below. A command that command_over_transport could +// not even BUILD (today, only RemoteExecInterpretationUnknown -- an SshExec whose remote shell +// dialect this runner refuses to assume) is NOT the same state as a command that reached a real +// process and exited nonzero: nothing ran in the first case, and treating it as though something +// ran and failed would erase that distinction. CommandOverTransportRefused therefore short-circuits +// to exit_failure directly and never reaches shell.Exec.RunArgv or the exit-code check at all. +// +// THE LocalExec ARM'S SHAPE BELOW IS THIS COLLAPSE'S OWN, not a preserved inheritance. Nothing +// about #8593's earlier LocalExec-only cut constrained what this run site had to look like once +// SshExec joined it through the same seam. func run_shell_command(command: ArgvCommand, transport: CommandTransport) -> ProcessExit uses net: Network { @@ -154,6 +179,31 @@ fn argv_operands(command: ArgvCommand) -> List { // wreckage. That is why a separate presence probe has to exist, and it is a boundary fact about // the transport rather than a limitation of this module. // +// THE BUILT/REFUSED DISTINCTION SURVIVES HERE TOO, and stays distinguishable from an ordinary +// failed run even though both terminate in the same ProcessRefused variant: a build refusal +// carries the sentinel exit_code: 0 - 1, which no real process exit code is, while a command that +// reached shell.Exec.RunArgv and exited nonzero carries that process's own exit_code. A caller +// matching on exit_code, not just the variant, can still tell "never ran" from "ran and failed". +// +// THE SPAWN-REFUSAL ASYMMETRY SURVIVES THE COLLAPSE, and it survives because the collapse never +// touched what program each arm names -- only how the resulting argv reaches the process. LocalExec +// names a CALLER-SUPPLIED program, so the spawn-failure arm above is reachable on it: a typo'd or +// missing binary refuses the evaluation, as described. SshExec's local-machine program is always +// sshpass/ssh (extdeps.exec.command ssh_exec_prefix) -- present by construction wherever an SshExec +// value exists at all -- so that arm cannot hit an unspawnable-local-program refusal the same way. +// Both facts are real properties of the two transports, not an accident of the old per-transport +// match; collapsing the run site onto one path must not read as making the arms symmetric where +// they are not. +// +// THE SSH EXIT-255 COLLISION IS ACCEPTED, NOT RESOLVED, and stays that way here: ssh overloads +// exit 255 for its own transport failures, indistinguishable by exit code alone from a remote +// command that legitimately exits 255 on its own terms, so both arrive here as one +// ProcessRefused { exit_code: 255, .. }. ProcessRefused.stderr carries ssh's message as prose +// ONLY -- readable by a human debugging a failure, never matched on by a caller, since a retry +// policy branching on unparsed prose would treat "host unreachable" and "command ran and exited +// 255" as the same case when they demand OPPOSITE responses. Full disposition, rung, and +// dissolution trigger: ssh_exit_255_conflation_dissolution, above. +// // WHAT THE CAPTURE SURFACE RETURNS, and why it is a coproduct rather than a record. // ShellCaptureResult carried success: Bool beside stdout: String, which made two states that a // caller cannot tell apart: a program that RAN AND FAILED printing nothing, and one that RAN AND