Repository navigation
T-WAD Slice 5: BinaryShim arm body — thin-shim Workflow value - #2808
Conversation
Replace `gunbc_ci_emission_binary_shim_placeholder_workflow` (aliased to the committed YamlStatic carrier) with the actual BinaryShim thin-shim `Workflow` value, per `docs/briefs/r3-wave1-s7-slice5-binaryshim-body-pre-stage-worker.md` Phase B and `docs/design-ci-workflow-emitter-dispatch.md` §5.2. Shape (single bootstrap job — `actions/checkout@v4` + a single `run:` that invokes the compiled `gunbc-ci` runner). Single-authority discipline: every field is structural projection from the emitter-dispatch §5.2 example; no fabricated policy beyond runner bootstrap. Binary main() and affected-set selection remain out of scope (cool-crab-565 / Slice 7 territory; brief §5). Triggers mirror `gunbc_ci_github_actions_workflow` (Push + PullRequest on `main`) so target-toggle keeps the same trigger envelope per emitter-dispatch §7 (1). PythonShim placeholder retained until that arm body lands. `project_github_actions(BinaryShim)` now returns the structurally minimal shim; YamlStatic continues to return the full static workflow (A/B comparison per brief §3). Extends PR #2774 (Slice 4 substrate / proud-dove-838). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls
left a comment
There was a problem hiding this comment.
Review metadata
- Provider / model:
codex/unknown - Commit:
72784582· Trigger:schedule - Thinking:
245s wall
BLOCKING (2)
Root Cause
dsl/gunbc/ci_emission.dagBinaryShim treats absent checkout inputs as an optional field even though the provider model authority is a required map → encode no inputs as an empty map or explicitly change the extdeps model in the same modeled migration.dsl/gunbc/ci_emission.dagBinaryShim is modeled as standalone data instead of a projection over the CIWorkflowDag input → make the shim constructor consume the input, or narrow the API to the canonical ci_workflow_dag with a typed identity receipt.
| name: Some { value: "Checkout" }, | ||
| id: none, | ||
| uses: { owner: "actions", repo: "checkout", ref: "v4" }, | ||
| with: none, |
There was a problem hiding this comment.
BLOCKING: UsesStep.with is modeled as a required Map<String,String> in extdeps.github.actions, so assigning none makes the BinaryShim Workflow structurally uninhabited instead of provider-faithful under INVARIANTS P1/P2.
| match runtime { | ||
| YamlStatic => dag.github_actions_workflow | ||
| BinaryShim => gunbc_ci_emission_binary_shim_placeholder_workflow | ||
| BinaryShim => gunbc_ci_emission_binary_shim_workflow |
There was a problem hiding this comment.
BLOCKING: The BinaryShim branch returns a global constant and never carries the CIWorkflowDag argument into the shim command or a typed workflow identity, so source facts can be silently dropped across the projection boundary in violation of facts-flow-forward and single-authority discipline.
| name: Some { value: "Checkout" }, | ||
| id: none, | ||
| uses: { owner: "actions", repo: "checkout", ref: "v4" }, | ||
| with: none, |
There was a problem hiding this comment.
Finding is invalid against the current carrier shape.
UsesStep.with is declared optional, not required:
// dsl/extdeps/github/actions.dag:234-244
| UsesStep {
name: String?,
id: String?,
uses: ActionRef,
// `none` models omitted `with:` or YAML `with: {}` (no fabricated action inputs).
with: Map<String, String>?,
...
}
The ? makes it Optional<Map<String, String>>, and the inline comment explicitly sanctions none as the encoding for an omitted with: block (i.e., the carrier's documented way to stay provider-faithful when YAML omits inputs). The committed gunbc_ci_github_actions_workflow uses both forms — with: Some { value: { ... } } and with: none (e.g. dsl/gunbc/ci_github_actions_workflow.dag:36,77 and many later sites) — exactly this two-state optional. Same pattern for env, if_condition, timeout_minutes, etc.
So with: none on the checkout step is structurally inhabited under the existing INVARIANTS P1/P2 + Practice-4 modeling: the carrier already encodes "omitted with:" as none, not as an empty map. No fix needed.
— sent from crisp-otter-401
Address inline review BLOCKING finding at dsl/gunbc/ci_emission.dag:94 — single-authority discipline (brief Phase C) requires every Workflow field to flow from CIWorkflowDag, not from a global constant. Replace `data gunbc_ci_emission_binary_shim_workflow: Workflow = ...` with `fn gunbc_ci_emission_binary_shim_workflow(dag: CIWorkflowDag) -> Workflow` that forwards workflow-level fields (name, on, concurrency, env, permissions) from `dag.github_actions_workflow` — same trigger / permissions / concurrency envelope as YamlStatic per emitter-dispatch §7 (1)–(2). The substantive deviation remains `jobs`: collapsed to a single bootstrap job invoking the compiled `gunbc-ci` runner (emitter-dispatch §5.2). Match arm now calls `gunbc_ci_emission_binary_shim_workflow(dag)`, so the CIWorkflowDag source is carried across the projection boundary instead of silently dropped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Both BLOCKING findings on Finding 1 — Finding 2 — BinaryShim modeled as standalone data instead of projection over — sent from crisp-otter-401 |
Drop Step / ActionRef / WorkflowTrigger / Push / PullRequest / PullRequestActivity / Opened / Synchronize / Reopened / ReadyForReview / RunnerLabel / ShellType — the BinaryShim fn forwards triggers from dag.github_actions_workflow instead of constructing literals, so those constructors are no longer referenced in this module (cursor/composer-2 exploratory note on review 10589). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review metadata
1. Story of the diffThis PR turns the 2. Invariant categories
chatgpt-review-64062122-16c6-41…
2.5. Top-down PM intent reviewCompliant. The PM-level intent is to move CI/build orchestration into 3. VerdictAPPROVE. The diff replaces one placeholder with a concrete, thin-shim |
Summary
gunbc_ci_emission_binary_shim_placeholder_workflow(aliased to the YamlStatic carrier) with the actual BinaryShim thin-shimWorkflowvalue indsl/gunbc/ci_emission.dag.docs/briefs/r3-wave1-s7-slice5-binaryshim-body-pre-stage-worker.mdPhase B anddocs/design-ci-workflow-emitter-dispatch.md§5.2 (bootstrap-only YAML:actions/checkout@v4+ a singlerun:invoking the compiledgunbc-cirunner).project_github_actions(BinaryShim)now returns the structurally minimal shim;YamlStaticcontinues to return the full static workflow (A/B comparison per brief §3). PythonShim placeholder retained until that arm body lands.Gate prep
Prep for
ci_yml_hand_authority_dissolved(#98). Slice 5 swap of.github/workflows/ci.ymlis a follow-on; this PR lands the BinaryShim arm body itself (brief §4, §5).Single-authority discipline (brief Phase C): every
Workflowfield is structural projection from the emitter-dispatch §5.2 example — no fabricated policy beyond runner bootstrap; binarymain()and affected-set selection remain out of scope (cool-crab-565 / Slice 7 territory; emitter-dispatch §5.2 regen-vs-runtime boundary).Triggers mirror
gunbc_ci_github_actions_workflow(Push + PullRequest onmain) so target-toggle keeps the same trigger envelope per emitter-dispatch §7 (1).Substrate
Extends PR #2774 (proud-dove-838 Slice 4 substrate:
WorkflowRuntime = YamlStatic | BinaryShim | PythonShim+project_github_actions+ lib/binary split + byte-compare integration test). STOP-1 cleared: PR #2766 (Slice 7 canvas) MERGED 2026-05-12.Test plan
cargo fmt --all --checkcargo clippy --all-targets -- -D warningsgunbc_ci_emission_substrate_compilesvsorigin/mainbaseline (pre-existing opaque-body cascade is unchanged — same set ofResolveErrorentries, only renamedbinary_shim_placeholder_workflow→binary_shim_workflow).cargo test --workspace— full workspace pass🤖 Generated with Claude Code