Repository navigation
srv3 host_effect_realize: typed argv over SSH/FleetSsh for the six surviving shell-string helpers - #8590
Conversation
…e srv3_join_shell_words Rewrites the six named srv3_* helpers in gunbc.host_effect_realize that built concatenated shell command strings for SshShell transport, and routes them (plus FleetSsh) through the existing typed_argv_exec_over_ssh / typed_argv_exec_over_fleet_ssh primitives instead. srv3_join_shell_words is deleted rather than ported, since string-joining argv is exactly what the typed path exists to avoid. realize_os_install_actuator_toolchain_ensure now admits FleetSsh at its entry gate (all five reachable probe leaves carry typed operation_argv rows, so the fabricated-negative concern that justified refusing FleetSsh at entry no longer applies). Corrects the now-stale doc block in gunbc.host_effect describing the leaf-migration population. Adds gunbc.extdeps.shell.shell_which_check_argv / shell_test_is_executable_argv (pure argv shapers, reused by the srv3 helpers) and a witness test covering the pure argv-construction helpers and the two typed-transport entry refusals; the FleetSsh dispatch arms themselves remain compile-verified only (no fixture route exists to a FleetSshExecutionContext, per the sealed-context doc note already in host_effect.dag). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
typed_argv_exec_over_{ssh,fleet_ssh} carries a bare argv with no shell to
cd first, so a non-root workdir over SSH/FleetSsh had no typed form to
route through. The prior workdir == "/" && ... guard folded that gap into
the success boolean directly, so a non-root workdir rendered as an
indistinguishable "false" -- the fabricated-negative shape DESIGN §5
forbids and the sibling EmitArtifactThenThinRun arm already names.
Every in-tree caller passes workdir: "/", so this stays unreachable dead
debt, but it now has a name and a note instead of being silent.
Addresses review 53851 on gunbc#8590 (non-blocking finding, fixed anyway).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… line The comment block preceding BmcwebTokenExtraction quoted jq's `.token // empty` filter verbatim, embedding a literal `//` inside `//`-prefixed prose. That is a corpus-unique pattern (grepped: zero other occurrences anywhere in dag/, std/, extdeps/) and it broke annotation-to-declaration attachment in the real claim_executor --required-floor compiler, which reported "source annotation names no subject" for the following lines and cascaded into ~100 not-found-in- scope errors once the module failed to parse -- the CI failure on PR #8590 @ 5ffca8d. Reworded to describe jq's alternative operator without spelling it literally in the comment text, preserving the original technical meaning (the operator is jq's `//`, not some jq built-in default). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ation
The real defect blocking CI was not the nested `//` in the comment block
(that fix was harmless but not load-bearing) -- it was
srv3_transport_witness_bin_nonroot_workdir_refused_note's string literal
containing the literal text "typed_argv_exec_over_{ssh,fleet_ssh}", added in
the prior review-fix commit. dag string literals treat `{` followed by an
identifier as interpolation syntax (DESIGN §4c / dag string literal rules),
so claim_executor's real parser read `{ssh,fleet_ssh}` as an interpolation
attempt, failed there, and the failure cascaded into the same "source
annotation names no subject" errors at lines 716-736 plus ~100 not-found-in-
scope errors once the module failed to parse -- confirmed from the actual
failing run's log: "host_effect_realize.dag:1060:72: error: in string
interpolation: a '{' followed by an identifier begins interpolation inside a
string literal; for a literal brace write \{".
Escaped it per the compiler's own suggested fix: `typed_argv_exec_over_\{ssh,fleet_ssh}`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Follow-up on the Checked what the tree actually declares about the srv1-4 targets these probes run against ( So per DESIGN §3 (a modeled operation forcing a less-safe realization is itself a modeling defect, not license to conform the call site to it): Not switching it in this PR: it is one call site, does not block this migration, and CI is green ( — sent from crisp-wren-896 |
… shell.PosixCommandV which is an external binary (debianutils) with no established presence on srv1-4 (fleet_intent_os_surface.distro_or_product: none, PR #8590 finding). command -v is a POSIX shell builtin, narrowing the unproven assumption from "a specific external binary is installed" to "a POSIX shell is on PATH" -- smaller and more portable, though still not a proof, and the carrier says so. The tool name is threaded through sh -c's own positional-parameter binding (argv: ["sh", "-c", "command -v \"$1\"", "sh", "{command}"]), the same pattern shell.Find.FilesAndSymlinksWithMode already uses, rather than quoted-and-concatenated into the script text: the value never touches the string parsed as shell code, so there is no escaping table to maintain and no quoting-authority dependency. $1 stays double-quoted inside the script to prevent word-splitting/globbing on the expanded value. Presence is read from exit_success only, never from parsing stdout text; stdout is read separately in srv3_tool_bin_path for the resolved path, which is a distinct and legitimate use. Other shell.Which.Check callers (bmc_fan_converge.dag, host_cli_dependency.dag, roadmap_belt_actuate.dag, interpreter_dispatch_bijection_real_roster_transport.dag) are out of scope and untouched. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mmandV.Check Two operations now answer "is tool X present," and nothing stated which one a new author should reach for -- the nicknaming shape, split by real distinction but with the distinction living only in a PR comment. Put the rule on shell.Which's own carrier, where the next author stands before calling it: Which is for targets whose package set is an established in-tree fact (with host_cli_dependency's apt-install incident as the example of what "established" means); PosixCommandV is for everything else, and always for a target whose OperatingSystemSurface carries distro_or_product: none with no other evidence. posix_command_v_semantic_note cross-references it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re review 53895 (APPROVE, non-blocking findings): Both are correctly identified as pre-existing patterns this PR grows rather than introduces (the — sent from crisp-wren-896 |
… failure)
Two prose notes on shell.PosixCommandV/shell.Which quoted `{command}` and
an OperatingSystemSurface literal `{ distro_or_product: none, ... }` with
bare braces, which .dag string interpolation reads as an expression --
CI's required-floor refused with "undefined variable 'command'" at
shell.dag:101:115. Escaped as \{ \} per the established literal-brace
convention.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
CI red on fcf67c3 was a self-inflicted parse defect, not a design issue: two carrier-prose strings ( — sent from crisp-wren-896 |
gunbc_ci_host carries the identical distro_or_product: none as srv1-4, so which's external-binary presence was never an established fact anywhere in this fleet -- command -v's weaker POSIX-shell-only assumption dominates which's everywhere: any target that can run which can run command -v, and the converse fails on busybox/stripped images. A selection rule between two operations is only worth its cost when each is right somewhere the other is wrong, so keeping both was the fork this lane exists to remove. Migrates all five live call sites (the fifth, extdeps.tools.resolve, was not in the original four-site classification -- found by grepping for the deleted service before deletion could land) off shell.Which.Check to shell.PosixCommandV.Check, deletes the service, its argv builder, its mock corpus entry, and the now-single-operation selection note. Verified none of the four .exists-only sites reads .path as a filesystem path, so the builtin-vs-external-binary stdout distinction (command -v prints the bare name for a builtin/alias, not a resolved path) does not bite there; resolve() does read .path and is flagged accordingly, with no live caller in the tree today. Records the modeling gap this rests on as its own finding at std.os.types: no OperatingSystemSurface instance anywhere in fleet_intent or ci_fleet populates distro_or_product, so no typed tool-availability question is answerable for any fleet host despite the *DistributionCatalogRow authority already existing -- the join from surface to catalog is what's missing. Also fixes a third instance (this lane's) of unescaped braces in .dag string-literal carrier prose tripping interpolation.
# Conflicts: # dag/gunbc/host_build_cache_provision_script.dag
…e denominator to its head The banner asserted that absence of a name is not discharge because a rename looks identical. It now demonstrates it, with six names that already have verdicts on main and do not all point the same way: srv3_chown_directory_to_current_user discharged 61bf47b (#8590) host_build_cache_provision_script discharged 40f2fb6 (#8825) four host_hygiene_reap_*_body files deleted ffa16a5 (#8583) git_fetch_script RENAME, not gone (unadjudicated) Three causes, one grep signature. The host_hygiene row is the sharpest: the commit that discharges those four names does not mention them, so the evidence is not reachable from the symbol at all -- which is precisely why the per-row lane cannot be done by symbol search. All four commits are ancestors of main, so the table is head-safe independent of the SS1.C corrections landing separately in #10529. The denominator now says it was measured at 70925ee and is valid only there: any later edit that strikes a row through changes what the closed-marker grep counts, so the 180/41/~139 split moves and must be re-derived rather than quoted. The demonstration table sits inside the blockquote, so it does not itself perturb the row count -- re-derived at 180 after the edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GCBVTWARwTLkAfVWaGfKA
… flag the census's dead-ci.yml population (#10537) * Correct the stale ci_floor_peak rows and flag the census's dead-ci.yml population gunbc.plans.shell_emission_model's Phase 1 PARTIAL row still said While emit's one production consumer, ci_floor_peak_emit, has cond and body "still raw shell strings". That was true when written and is stale: ecbd086 (#7978, shell -> dag Phase 1 cgroup vertical, 2026-08-08, ancestor of origin/main) replaced both leaves, and the membership assign with them, with derived text from a typed NearestAncestorContaining (std.path_intent) plus extdeps.linux.proc_self_cgroup membership, lowered through bash_orch_if / bash_proc_self_cgroup, refusing with a located marker rather than widening. git log -S on both bash_nearest_ancestor_locate_cond_command and ci_floor_peak_nearest_ancestor_spec, scoped to that file, returns exactly that commit as first introduction. The correction carries the residue forward rather than reading as completion: the peak calibration leaves on the same pipeline and the runtime-scan transport in bash_membership_assign_from_source are still raw String Run.command values, declared Phase 2 typed filesystem observation by the module itself. Three census rows in docs/plans/shell-to-dag-residual-census-and-arc-completion.md carried the same stale claim -- the SS1.A "concat-built floor-peak/cgroup runners" row, the SS4.E already-on-emit row, and the SS4.J Phase-1 dispatch row that still listed the two leaves as unassigned open work. That last one is what dispatched a lane onto discharged work. Sweeping the class turned up a larger root cause, filed as a banner rather than silently truncated: .github/workflows/ci.yml was deleted by 611fd02 (#8283, FLOOR-Y cutover), CI is now the witnesses.yml emission from gunbc.witness_floor_workflow, and this census mentions none of that while still naming ci.yml drift+parse as its byte-oracle in four places. A symbol-existence check over the SS4.J dispatch roster found ~20 named production symbols with no declaration anywhere in the .dag corpus. The banner states the denominator (180 data rows, 41 already closed, ~139 asserting live state) and states explicitly what it does NOT establish: absence of a name is not discharge, since a rename looks identical -- git_fetch_script is gone while git_fetch_no_tags_shell and git_fetch_prune_shell exist. Per-row adjudication needs its own lane. Model-side prose only; no seed growth, no .rs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GCBVTWARwTLkAfVWaGfKA * Name the instrument behind the banner's counts, so the banner is not the next stale row The staleness banner cited bare counts (180 data rows / 41 closed / ~139 live) and a ~20-name absent-symbol list. Transcribed numbers under a dated claim are exactly the defect the banner is about, one layer up: nothing re-derives them, so they rot without anyone touching either end. The banner now carries the two checks that produce them, verified to reproduce their stated output as printed at 70925ee, and says to run them rather than read them. It also states the second check's raw output honestly -- 50 candidates, of which the ~20 production symbols are what survives discarding module names, the commit-sha and fn-fragment spellings the banner itself introduced, and already-struck rows -- so the gap between "50 out" and "20 listed" cannot be mistaken for a miscount. The ci.yml deletion is the one claim that is not count-shaped and its own one-line derivation is given. The dispatch prohibition is unchanged and deliberate: a banner that describes staleness while still permitting dispatch off the rows is a documented hazard with no refusal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GCBVTWARwTLkAfVWaGfKA * Turn the banner's absent-symbol list into a demonstration, and pin the denominator to its head The banner asserted that absence of a name is not discharge because a rename looks identical. It now demonstrates it, with six names that already have verdicts on main and do not all point the same way: srv3_chown_directory_to_current_user discharged 61bf47b (#8590) host_build_cache_provision_script discharged 40f2fb6 (#8825) four host_hygiene_reap_*_body files deleted ffa16a5 (#8583) git_fetch_script RENAME, not gone (unadjudicated) Three causes, one grep signature. The host_hygiene row is the sharpest: the commit that discharges those four names does not mention them, so the evidence is not reachable from the symbol at all -- which is precisely why the per-row lane cannot be done by symbol search. All four commits are ancestors of main, so the table is head-safe independent of the SS1.C corrections landing separately in #10529. The denominator now says it was measured at 70925ee and is valid only there: any later edit that strikes a row through changes what the closed-marker grep counts, so the 180/41/~139 split moves and must be re-derived rather than quoted. The demonstration table sits inside the blockquote, so it does not itself perturb the row count -- re-derived at 180 after the edit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GCBVTWARwTLkAfVWaGfKA * Re-derive on the merged tree: correct a worked example reading found, repin the denominator Reading every row either PR touched on the merged tree -- rather than trusting the clean auto-merge -- turned up an error in my own worked-examples table. #10529's srv3 row names a live successor, srv3_ensure_directory_owned_by_current_user (gunbc.host_effect_realize), so srv3_chown_directory_to_current_user was not plain "discharged" as my table said: it was RENAMED AND CLIMBED at once. That is a fourth cause a two-way discharge/rename split cannot express, and calling it discharged loses the successor a reader needs to find. The table now carries all four causes: dissolution, file deletion upstream of the name, bare rename, and rename-plus-climb. The denominator moved exactly as predicted. Re-derived on the merged tree it is 180 rows / 43 closed / ~137 live, against 41/~139 one merge earlier; #10529 striking two more SS1.C rows through is the whole difference. The banner now says so, because a split that moves within one merge of being written is the argument for naming the recipe rather than a bookkeeping detail. Measurement is pinned to the merge of 70925ee with origin/main 16a702e. Controls re-run on the merged tree, not carried forward: the four bash_nearest_ancestor_locate_* call sites still stand and #7978 is an ancestor of the merged HEAD; ci.yml is still absent with witnesses.yml present and #8283 an ancestor; git_fetch_script is still absent while git_fetch_no_tags_shell and git_fetch_prune_shell exist, so the rename example still discriminates. Diff is still model-side prose only -- no .rs, no seed growth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013GCBVTWARwTLkAfVWaGfKA --------- Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Summary
srv3_*helpers ingunbc.host_effect_realize(srv3_transport_witness_bin_success,srv3_transport_test_executable,srv3_apt_tool_present,srv3_tool_bin_path,srv3_chown_directory_to_current_user, plus the transitively-touchedsrv3_join_shell_words) off concatenated shell command strings and onto the existing typedtyped_argv_exec_over_ssh/typed_argv_exec_over_fleet_sshprimitives.srv3_join_shell_wordsis deleted (not ported) — the typed argv path exists precisely to avoid re-joining aList<String>into a string.realize_os_install_actuator_toolchain_ensurenow admitsFleetSshat its entry gate: all five reachable probe leaves carry typedoperation_argvrows, so the fabricated-negative-observation concern that previously justified refusing FleetSsh at entry no longer applies.gunbc.host_effectdescribing the leaf-migration population (it previously claimed ten remaining call sites across two files; the live corpus had five, all inhost_effect_realize).extdeps.shell(shell_which_check_argv,shell_test_is_executable_argv), reused by the srv3 helpers instead of re-deriving the same argv shape inline.dag/test/claim/srv3_host_effect_realize_witness_test.dag, covering the pure argv-construction helpers and the two typed-transport entry refusals (BmcController target, EmitArtifactThenThinRun transport).Scope notes
FleetSshExecutionContextis reachable only throughmaterialize_fleet_known_hosts_anchor, afuncusingNetworkbehind theKnownHostsAnchorseal, so a puretest fncannot construct one. This is a pre-existing, structural limitation of the seal (documented inline inhost_effect_realize.dag), not something this PR papers over.Test plan
dag/test/claim/srv3_host_effect_realize_witness_test.dagadded, covering pure argv shaping (srv3_argv_with_bin,srv3_actuator_toolchain_chown_args,shell_which_check_argv,shell_test_is_executable_argv) and the two entry-levelNotConvergedrefusal paths.claim_executor --required-floor) on this PR.Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com