Repository navigation
Emit lane: empty then_/else_ arms emit bash no-op ':' instead of invalid syntax - #7282
Merged
Merged
Conversation
added 5 commits
July 26, 2026 06:08
5 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #7277.
Pipeline.steps(v2.std.orchestration) is a plainFreeMonoid<PipelineStep>with noNonEmptyvariant and no smart constructor, so a zero-stepPipeline(steps: Empty) is legally constructible today. Nested as anIf'sthen_/else_arm,orch_emit_if_stepthreaded that arm's emitted""straight into the inline bash row's body slot, producing"if COND; then ; fi"— invalid bash (athenclause needs ≥1 command before the closing separator; real bash raises "syntax error near unexpected token `fi'"). This is a fabricated-invalid-output shape, not a refusal.Zero production occurrences of an empty arm exist in the tree today (verified via
Pipeline { steps: ... }census acrossdag/gunbcandsrc/v2/workflow) — this is a priced, cheap fix ahead of any real trigger, not a live incident.Fix (uniform, no special-casing): an empty arm's body always emits the bash no-op
:(orch_if_arm_body_or_noopin05_emit_orchestration.dag), applied identically in the inline and block dispatch and regardless of whether the other arm is also empty. This covers four shapes with one rule:then_,else_Absentthen_, non-emptyelse_present ("act only on the else")then_, emptyelse_present ("act only on the then")Why no refusal for the both-empty case (reconsidered mid-review):
Predicate(ExitZero/LogMatches/StrEq/etc.) executes a command — so even a both-emptyIfruns the condition and discards the outcome, a real "run cmd, ignore result" intent, not dead code. Refusing it would forbid working intent. This also matches the existing convention already in the codebase: a top-level emptyPipeline(not nested in anIf) already emits""cleanly viaorch_emit_pipeline'sHeadAbsentarm, with no error — so "empty pipeline is a legitimate no-op" was already this codebase's convention before this PR; the fix makes that convention hold uniformly whether or not thePipelineis nested inside anIf.Option (3) declined, named on the carrier: a
NonEmpty<PipelineStep>carrier onPipeline.stepswas considered (construction-first, §5-preferred shape in the abstract) and declined — zero occurrences in tree today, and it would touchFreeMonoid<PipelineStep>plus everyPipelineconstructor for a currently-unexercised case (the purity trap, DESIGN.md §6). This is recorded as a typed note directly on the carrier:pipeline_steps_empty_arm_notebesidePipeline's declaration insrc/v2/std/orchestration.dag, plusorch_if_empty_arm_notebeside the fix in05_emit_orchestration.dag— not just in this PR body, per DESIGN §6 "let the mark on the carrier be the authority."Test plan
Four new discriminating RED witnesses in
src/v2/workflow/orchestration_emit_test.dag, all asserting valid emitted bash (none asserting a refusal):orch_if_empty_then_no_else_noop_holds→"if [ -z \"$1\" ]; then :; fi"orch_if_empty_then_with_nonempty_else_noop_holds→"if [ -z \"$1\" ]; then :; else verdict=present; fi"orch_if_nonempty_then_with_empty_else_noop_holds→"if [ -z \"$1\" ]; then verdict=absent; else :; fi"orch_if_both_arms_empty_noop_holds→"if [ -z \"$1\" ]; then :; else :; fi"Run via:
All 4 PASS.
Byte-identity receipt (post-#7277 main)
Re-ran against main post-#7277-merge (this branch merged main via
git merge, not rebase) to confirm the fix doesn't disturb any existing emitted bytes:Ifcall-site witnesses (the 3ci_floor_peak_emit.dagsingle-step sites covered byci_floor_peak_emit_band_holds, and the 2live_deploy/emit.dagno-else sites covered bylive_deploy_emit_holds) — PASS, byte-identical.Ifshape tests inorchestration_emit_test.dag(orch_if_single_step_no_else_byte_identical_holds,orch_if_single_step_with_else_byte_identical_holds, and the 3 multi-step block-form tests from Emit lane: block-bodied if_else row for multi-step then_/else_ bind #7277) — PASS, byte-identical.Combined run (11 witnesses total), exit code 0, all PASS: