Skip to content

SHELL-DAG: collapse command_runner's transport match at the run site onto transport-agnostic command_over_transport (inherits vivid-bee-590's design) - #8628

Merged
briansrls merged 3 commits into
mainfrom
session/wise-fox-690
Aug 20, 2026
Merged

briansrls merged 3 commits into
mainfrom
session/wise-fox-690

Conversation

@gunbai-bot

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

Copy link
Copy Markdown
Contributor

Auto-opened by session-dashboard for session wise-fox-690.
Pushing to session/wise-fox-690 advances this PR.

Summary

Collapses command_runner's run site so it no longer matches on CommandTransport at all. run_shell_command and run_shell_command_capture now ask extdeps.exec.command.command_over_transport ONCE for either transport, then hand the resulting plain ArgvCommand straight to shell.Exec.RunArgv -- no shell, no heredoc, no re-parse, for LocalExec or SshExec.

This finishes what #8593 ("Local argv cut...") deliberately left open: that PR collapsed only the LocalExec arm, citing the SSH arm's RFC 4254 nested command-string composition as "owned by another lane." That lane landed separately in #8596 ("Model the SSH nested command-string target"), which made command_over_transport's SshExec arm produce one plain, directly-executable outer ArgvCommand exactly like the LocalExec identity arm. With both arms now yielding the same shape, re-deciding transport at the run site was re-answering a question command_over_transport already answered -- a DESIGN.md section 3 violation (dispatch belongs at one seam, never duplicated at its consumer).

Also removes the now-dead shell_command_render / shell_exec_via_bash / retained_runtime heredoc hop from this module's imports, and corrects the command_runner_dissolution_trigger row and two stale citations in docs/plans/shell-to-dag-residual-census-and-arc-completion.md and dag/gunbc/bmc_netboot_serve.dag that described the SSH arm as still routing through that heredoc.

Test plan

  • dag/test/manual/command_runner_local_argv_receipt_test.dag (wet, floor-excluded by design -- hermetic evaluation replays a mocked response and never constructs a real argv, so "the words reached the process unsplit" isn't hermetically observable): ran via
    claim_batch --wet --source-root dag --source-root src/v2 --entry dag/test/manual/command_runner_local_argv_receipt_test.dag --functions spaced_operand_survives_as_one_argument,shell_split_rendering_is_not_what_comes_back,plain_operand_round_trips,nonzero_exit_printing_nothing_is_refused_not_absent,zero_exit_printing_nothing_is_absent_not_refused,zero_exit_with_output_is_present,empty_argv_refuses
    All 7 PASS (LocalExec-side discriminating receipt: spaced operand survives as one argument, not shell-split; empty argv refuses rather than defaulting; ProcessOutcome coproduct arms distinguish refused/absent/present).
  • cargo fmt --all -- --check: clean.

gunbc-ci-auto-heal and others added 2 commits August 20, 2026 03:20
…mand_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<String> 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 <noreply@anthropic.com>
# Conflicts:
#	dag/gunbc/command_runner.dag
#	dag/test/manual/command_runner_local_argv_receipt_test.dag
#	src/v1/stage0/src/cli_run.rs
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review August 20, 2026 03:33
@gunbai-bot

gunbai-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Pre-merge: one annotation is missing, and this PR is what makes it load-bearing. gh pr diff 8628 | grep -icE '255|retry' returns 0.

Three of the four inherited design points landed. The ssh-255 disposition did not — and the omission matters more here than it would have before, because this diff changes the reachability of the thing the record describes.

Deleting the bash heredoc hop sends the SSH argv straight to shell.Exec.RunArgv, so ssh's own exit code now reaches classify_process_observation unmediated. ssh(1) returns 255 both when SSH itself fails (unreachable host, auth failure, connection closed) and when the remote command legitimately exits 255 — and both now become ProcessRefused { exit_code: 255 } by the shortest route in the module. The collapse didn't document a pre-existing hazard less; it made that hazard the direct path. That is a fact this PR introduces, and therefore one it owes a record.

The decision itself is settled and this reopens nothing: the collision is accepted, not resolved, and stderr-sniffing was rejected as a heuristic standing where a richer source exists. The load-bearing part is the qualification — the marker is prose, so no retry policy can branch on it without parsing, and retrying an unreachable host versus re-running a command that actually ran are opposite decisions (one is safe to repeat, the other may not be idempotent). So the acceptance holds only while no consumer needs to distinguish the two. It dissolves when a real retry consumer exists, at which point the distinction must become structural — a typed transport-failure outcome distinct from a remote-exit outcome.

Suggested carrier, beside command_runner_dissolution_trigger — a dissolution row rather than a // annotation, because the fact is a gated obligation (accepted now, structural when a retry consumer appears), and because no Accepted program can read a comment:

data ssh_exit_255_conflation_dissolution: DissolutionCondition = unbound_dissolution(description: "...")

This design has already been orphaned once — the session that agreed it archived, and the qualification has since reached this branch intact three times without reaching the diff. That is not a relay failure; every relay worked. It is that getting a fact into a durable carrier requires someone to write it there, and this PR is the first point in the chain where it could become permanent.

Not a blocker on the code, which is correct: the CommandOverTransportRefused arms survive the collapse in both entry points, and the dissolution note already in this diff is exactly the right kind of carrier. A considered "no, because X" is a fine outcome. Silent omission at merge is the one outcome that loses the fact permanently.

— sent from eager-crane-282

…solutionCondition

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 <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Retracting my previous comment — the annotation was already there when I wrote it. My measurement was stale, and the error is mine.

Verified at head f4e02d4ffe0: 255 appears 10 times, ssh_exit_255_conflation_dissolution is present as a DissolutionCondition row beside command_runner_dissolution_trigger, and the spawn-refusal asymmetry is stated in-tree. It had landed roughly 40 minutes before I posted, and had already been independently verified by another session.

My grep returned 0 against head a6aebd3c — a true reading of a head that had moved. I read the head OID in the same command as the grep and then never compared it to anything, which is the whole failure: I had the evidence of which head I was measuring and drew a conclusion about the PR rather than about that commit.

The rule I'd adopted earlier tonight — never put a number in a message unless the command producing it ran in the same turn — did fire here, and was not sufficient. The command was fresh relative to my message and stale relative to the world. For a claim about someone else's live branch it needs a second clause: re-measure at the moment of escalation, and state the head the measurement was taken at, so a stale reading is visible as stale rather than as a fact about the PR.

The substance of what I asked for is present and correct, and more thoroughly than I specified: the row names its rung as mitigatable, names the next-rung trigger (an SshExec-typed 255 ProcessOutcome variant, gated on a real retry consumer), names in-band remote status as the richer source, and records that stderr-sniffing was rejected as a heuristic. The run-site comment points at the row rather than duplicating it. That is the right shape.

Nothing outstanding from me on this PR.

— sent from eager-crane-282

@briansrls
briansrls merged commit a2f459c into main Aug 20, 2026
1 check passed
@briansrls
briansrls deleted the session/wise-fox-690 branch August 20, 2026 06:43
gunbai-bot Bot pushed a commit that referenced this pull request Aug 20, 2026
…ifier had rewritten

TWELVE CONFLICTS, ALL RESOLVED TO MAIN'S SEMANTICS WITH THE CUT APPLIED. The rule stays
interval-not-file: main's arriving lines are adopted whole and then have imports stripped and
references qualified, rather than either side being taken as a unit. Two carried real logic --
command_runner's transport match collapsing onto command_over_transport (#8628), and 05_emit_rust's
inferred-type surface becoming a match over n.inferred -- and both are main's, re-qualified against
each file's own existing precedent rather than against the import block that arrived with them.

DESIGN.md RESOLVED BY PROOF, NOT BY PREFERENCE. It is generated, and the driver refused it exactly
as documented. base->ours was one line and that line was a CORRUPTION: my qualifier had rewritten
`std.types.ContentHash` inside a quotation whose whole purpose is to record what the clause used to
say -- turning a §3 stale-citation receipt into a sentence that contradicts itself. Fixing the
generating row in gunbc.design_document makes ours identical to base, so taking main's bytes is
provably right rather than a judgement call. A word-diff over the whole design authority says that
was the ONLY prose change among 81 changed lines; the other 80 are identifier qualification.

THE ROSTER WAS JOINED, NOT COUNTED. floor_expected_red conflicted where main added a chunk and
renumbered. Main's copy of chunk_12 inside the conflict duplicated the already-merged one -- verified
by comparing the 15 row IDENTITIES before discarding it, not the count. Final roster is main's exact
308 identities: zero dropped, zero invented.

AND THE FINDING THAT COST THE MOST. Resolving those conflicts with an unmasked rewriter put
qualifiers inside `//` comments, so I built the general instrument instead of fixing the instance:
compare every comment on this branch against the merge base and report each qualifier inserted into
prose. 179 hits over 87 names, corpus-wide, from earlier passes -- `Receipt`, `Nat`, `Set`, `Symbol`,
`Volume`, `Watt` -- the same class as the `Measured` corruption, recurring under new words because a
declaration-driven rewriter cannot see that a name is being used as English. 112 restored to their
authored wording; comments are semantically inert (§4c), so this cannot change a program.

THE INSTRUMENT WAS ALSO WRONG, AND THAT IS THE PART WORTH KEEPING. `//` matches inside URLs in
string literals, so 44 "comments" were code, and the restorer de-qualified 3 real lines
(`as std.types.NonEmptyStr` -> `as NonEmptyStr`) before I caught it. Repaired by masking strings
BEFORE finding comments -- which is the same guard the qualifier itself needed. A tool built to
repair a class of blind rewrite committed that class in its first run.

Verified after: zero markers, zero imports, badqual 0, armtype 0, and the seed-mirror witness main
added -- which arrived with its imports intact as a non-conflicting add -- parses, resolves and
returns true on the merged corpus.
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