BT-E1: fix cross-module transport node deduplication

core/daglang/daglang-lower/src/lib.rs

@@ -6023,13 +6023,13 @@ fn add_service_call_edges(
     known_interface_types: &HashSet<String>,
     data_values: &HashMap<String, serde_json::Value>,
 ) -> Result<(), LowerError> {
+    // Track transport endpoint usage across ALL modules and callables so
+    // that the second caller to reference the same service operation gets
+    // a cloned triplet (_c1, _c2, …) instead of wiring duplicate scalar
+    // edges to the original.
+    let mut endpoint_use_count: HashMap<String, usize> = HashMap::new();
     for module in &project.modules {
         let module_name = module.module_path.as_dotted();
-        // Track transport endpoint usage across ALL callables in the module so
-        // that the second callable to reference the same service operation gets
-        // a cloned triplet (_c1, _c2, …) instead of wiring duplicate scalar
-        // edges to the original.
-        let mut endpoint_use_count: HashMap<String, usize> = HashMap::new();
         for item in &module.ast.items {
             let (item_name, params, stmts, uses_binding_types, body_lossy) = match &item.node {
                 Item::FnDef(def) => (

core/daglang/daglang-lower/src/tests.rs

@@ -3020,6 +3020,74 @@ func dual(msg: String) -> { reply: String } {
     );
 }
 
+#[test]
+fn cross_module_service_dedup_clones_transport_triplet() {
+    // Two modules in the same project that each call the same service
+    // operation must each get their own transport triplet (original + _c1).
+    // This is the cross-MODULE variant of the cross-callable test above —
+    // it regresses BT-E1 where endpoint_use_count reset per module.
+    let typed = typed_project_from_sources(&[
+        (
+            "dsl/services/dedup_cross_mod_svc.dag",
+            r#"module sample.dedup_cross_mod_svc
+service Echo {
+  operation Ping(message: String) -> { reply: String } {
+    transport shell { argv: ["echo", "{message}"] }
+  }
+}"#,
+        ),
+        (
+            "dsl/tools/dedup_cross_mod_a.dag",
+            r#"module sample.dedup_cross_mod_a
+import sample.dedup_cross_mod_svc { Echo }
+func alpha(msg: String) -> { reply: String } {
+  result = Echo.Ping(message: msg)
+  return { reply: result.reply }
+}"#,
+        ),
+        (
+            "dsl/tools/dedup_cross_mod_b.dag",
+            r#"module sample.dedup_cross_mod_b
+import sample.dedup_cross_mod_svc { Echo }
+func beta(msg: String) -> { reply: String } {
+  result = Echo.Ping(message: msg)
+  return { reply: result.reply }
+}"#,
+        ),
+    ]);
+    let dag = lower_typed_project(&typed).expect("lowering should succeed");
+    let prepare_nodes: Vec<_> = dag
+        .nodes
+        .iter()
+        .filter(|n| n.id.0.starts_with("prepare_transport_") && n.id.0.contains("Echo_Ping"))
+        .collect();
+    assert!(
+        prepare_nodes.len() >= 2,
+        "expected at least 2 prepare nodes for Echo.Ping (original + clone), found {}: {:?}",
+        prepare_nodes.len(),
+        prepare_nodes.iter().map(|n| &n.id.0).collect::<Vec<_>>()
+    );
+    // Verify no two edges target the same scalar (node, port) from
+    // different sources — the original duplicate-edge bug (BT-E1).
+    let mut edge_targets: std::collections::HashMap<(String, String), Vec<String>> =
+        std::collections::HashMap::new();
+    for edge in &dag.edges {
+        edge_targets
+            .entry((edge.to_node.0.clone(), edge.to_port.0.clone()))
+            .or_default()
+            .push(edge.from_node.0.clone());
+    }
+    for ((node, port), sources) in &edge_targets {
+        if port == PortName::DEPS || port == "request" || port == "response" {
+            continue; // these are allowed to have multiple sources
+        }
+        assert!(
+            sources.len() <= 1,
+            "duplicate scalar edge to {node}:{port} from {sources:?}",
+        );
+    }
+}
+
 // ── Pattern expansion tests ──────────────────────────────────────
 
 /// Lower a synthetic test module injected into the real DSL corpus.

gunbc-dag/src/makegen/registry.rs

@@ -233,22 +233,6 @@ pub struct ExtraTarget {
     pub post_commands: Vec<String>,
 }
 
-impl ExtraTarget {
-    /// Create a new extra target.
-    pub fn new(suffix: impl Into<String>, description: impl Into<String>) -> Self {
-        Self {
-            suffix: suffix.into(),
-            description: description.into(),
-            post_commands: Vec::new(),
-        }
-    }
-
-    /// Add a post command.
-    pub fn with_command(mut self, cmd: impl Into<String>) -> Self {
-        self.post_commands.push(cmd.into());
-        self
-    }
-}
 
 /// An entrypoint parameter that becomes a Make variable.
 #[derive(Debug, Clone)]
@@ -267,36 +251,6 @@ pub struct EntrypointParam {
     pub repeatable: bool,
 }
 
-impl EntrypointParam {
-    /// Create a new entrypoint parameter.
-    pub fn new(
-        port_name: impl Into<String>,
-        make_var: impl Into<String>,
-        cli_flag: impl Into<String>,
-        type_hint: impl Into<String>,
-    ) -> Self {
-        Self {
-            port_name: port_name.into(),
-            make_var: make_var.into(),
-            cli_flag: cli_flag.into(),
-            type_hint: type_hint.into(),
-            default: None,
-            repeatable: false,
-        }
-    }
-
-    /// Set a default value.
-    pub fn with_default(mut self, default: impl Into<String>) -> Self {
-        self.default = Some(default.into());
-        self
-    }
-
-    /// Mark as repeatable (for list parameters).
-    pub fn repeatable(mut self) -> Self {
-        self.repeatable = true;
-        self
-    }
-}
 
 /// Minimal workflow descriptor for tool targets.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -337,85 +291,6 @@ pub struct ToolInfo {
 }
 
 impl ToolInfo {
-    /// Create a tool in its own package.
-    pub fn new(
-        binary: impl Into<String>,
-        short_name: impl Into<String>,
-        description: impl Into<String>,
-    ) -> Self {
-        Self {
-            invocation: CargoInvocation::new(binary),
-            short_name: short_name.into(),
-            description: description.into(),
-            entrypoints: Vec::new(),
-            extra_targets: Vec::new(),
-            has_declarative_dag: false,
-            needs_generated_cli: true,
-            live_secrets: Vec::new(),
-        }
-    }
-
-    /// Create a tool that's a binary inside another package.
-    pub fn in_package(
-        binary: impl Into<String>,
-        package: impl Into<String>,
-        short_name: impl Into<String>,
-        description: impl Into<String>,
-    ) -> Self {
-        Self {
-            invocation: CargoInvocation::in_package(binary, package),
-            short_name: short_name.into(),
-            description: description.into(),
-            entrypoints: Vec::new(),
-            extra_targets: Vec::new(),
-            has_declarative_dag: false,
-            needs_generated_cli: true,
-            live_secrets: Vec::new(),
-        }
-    }
-
-    /// Create a standalone tool from its component name.
-    pub fn standalone(component: &str, description: impl Into<String>) -> Self {
-        Self {
-            invocation: CargoInvocation::standalone(component),
-            short_name: component.to_string(),
-            description: description.into(),
-            entrypoints: Vec::new(),
-            extra_targets: Vec::new(),
-            has_declarative_dag: false,
-            needs_generated_cli: true,
-            live_secrets: Vec::new(),
-        }
-    }
-
-    /// Create a tool that is a repo-local workspace binary.
-    pub fn workspace(tool_name: &str, description: impl Into<String>) -> Self {
-        Self {
-            invocation: CargoInvocation::composed(tool_name, "dag"),
-            short_name: tool_name.to_string(),
-            description: description.into(),
-            entrypoints: Vec::new(),
-            extra_targets: Vec::new(),
-            has_declarative_dag: false,
-            needs_generated_cli: true,
-            live_secrets: Vec::new(),
-        }
-    }
-
-    /// Create a tool that lives inside another package.
-    pub fn composed(component: &str, package_component: &str, description: impl Into<String>) -> Self {
-        Self {
-            invocation: CargoInvocation::composed(component, package_component),
-            short_name: component.to_string(),
-            description: description.into(),
-            entrypoints: Vec::new(),
-            extra_targets: Vec::new(),
-            has_declarative_dag: false,
-            needs_generated_cli: true,
-            live_secrets: Vec::new(),
-        }
-    }
-
     /// Create a ToolInfo from a codegen ToolDef.
     pub fn from_tool_def(def: &gunbc_codegen::registry::ToolDef) -> Option<Self> {
         let invocation = def.invocation.as_ref()?;
@@ -451,30 +326,6 @@ impl ToolInfo {
         &self.invocation.binary
     }
 
-    /// Add an entrypoint parameter.
-    pub fn with_param(mut self, param: EntrypointParam) -> Self {
-        self.entrypoints.push(param);
-        self
-    }
-
-    /// Add an extra composite target.
-    pub fn with_extra_target(mut self, target: ExtraTarget) -> Self {
-        self.extra_targets.push(target);
-        self
-    }
-
-    /// Mark this tool as having a declarative DAG definition.
-    pub fn with_declarative_dag(mut self) -> Self {
-        self.has_declarative_dag = true;
-        self
-    }
-
-    /// Mark this tool as having a hand-written main.rs (no generated CLI).
-    pub fn manual(mut self) -> Self {
-        self.needs_generated_cli = false;
-        self
-    }
-
     /// Build a normalized workflow specification for this tool target.
     pub fn workflow_spec(&self) -> WorkflowSpec {
         WorkflowSpec {
@@ -653,17 +504,53 @@ mod tests {
     #[test]
     fn tools_needing_codegen_reflects_flag() {
         let mut registry = ToolRegistry::new();
-        registry.register(ToolInfo::workspace("generated", "Generated tool"));
-        registry.register(ToolInfo::workspace("manual", "Manual tool").manual());
+        registry.register(ToolInfo {
+            invocation: CargoInvocation::composed("generated", "dag"),
+            short_name: "generated".to_string(),
+            description: "Generated tool".to_string(),
+            entrypoints: Vec::new(),
+            extra_targets: Vec::new(),
+            has_declarative_dag: false,
+            needs_generated_cli: true,
+            live_secrets: Vec::new(),
+        });
+        registry.register(ToolInfo {
+            invocation: CargoInvocation::composed("manual", "dag"),
+            short_name: "manual".to_string(),
+            description: "Manual tool".to_string(),
+            entrypoints: Vec::new(),
+            extra_targets: Vec::new(),
+            has_declarative_dag: false,
+            needs_generated_cli: false,
+            live_secrets: Vec::new(),
+        });
         let codegen = registry.tools_needing_codegen();
         assert_eq!(codegen.len(), 1);
         assert_eq!(codegen[0].short_name, "generated");
     }
 
     #[test]
     fn workflow_spec_for_tool_uses_codegen_dependency_when_needed() {
-        let generated = ToolInfo::workspace("alpha", "Alpha");
-        let manual = ToolInfo::workspace("beta", "Beta").manual();
+        let generated = ToolInfo {
+            invocation: CargoInvocation::composed("alpha", "dag"),
+            short_name: "alpha".to_string(),
+            description: "Alpha".to_string(),
+            entrypoints: Vec::new(),
+            extra_targets: Vec::new(),
+            has_declarative_dag: false,
+            needs_generated_cli: true,
+            live_secrets: Vec::new(),
+        };
+        let manual = ToolInfo {
+            invocation: CargoInvocation::composed("beta", "dag"),
+            short_name: "beta".to_string(),
+            description: "Beta".to_string(),
+            entrypoints: Vec::new(),
+            extra_targets: Vec::new(),
+            has_declarative_dag: false,
+            needs_generated_cli: false,
+            live_secrets: Vec::new(),
+        };
         let generated_spec = generated.workflow_spec();
         let manual_spec = manual.workflow_spec();
         assert_eq!(generated_spec.deps, vec!["ensure-codegen"]);

tasks.md

@@ -120,7 +120,7 @@ L4+ needs transport on all services the local profile touches — BT6 handles th
 | 13 | BT12 | **ArtifactStore providers.** `gcs_artifact_store.dag` (cloud_run) + `inline_artifact_store.dag` (local). Transport blocks, test blocks, profile bindings. | L8 | M | Done | BT10 |
 | 14 | BT-R2 | **SDLC review: provider completion.** Transport blocks on gcs_claim_store + gcs_outcome_ledger. Inline definitions extracted to provider files. Dead code deleted from pipelines/sdlc.dag. deploy.dag variable naming fixed. Profile bindings updated in sdlc.dag. | — | M | Done | BT11, BT12 |
 | 15 | BT-R3 | **SDLC review: fix 3 execution gaps.** LLM mock responses (enriched `default_rest_response` with LLM-shaped fields), `navigate_json_path` `/` separator + array index support, auth credential embedding in `GenericRestPrepareOp`, `CallParamSourceOp` replaces `IdentityCallableOp` for param_source nodes, param_source propagation in sdlc.rs CLI. Design: `docs/design/mock-response-pipeline.md`. | — | M | Done | BT10 |
-| 16 | BT-E1 | **Transport node deduplication.** `gunbc-sdlc --dry-run` fails at 408/494 nodes: `scalar input 'prepare_transport_...Anthropic_Messages.max_tokens' has multiple upstream edges`. Root cause: lowerer creates ONE shared transport triplet per service operation, but `endpoint_use_count` resets per module — callables in different modules both wire literal sources to the same prepare node's scalar port. Fix: make `endpoint_use_count` global across all modules in the compiled graph (not per-module). Touches: `daglang-lower/src/lib.rs` (`add_service_call_edges`, line 5683). | L1 | M | Pending | BT-R3 |
+| 16 | BT-E1 | **Transport node deduplication.** `gunbc-sdlc --dry-run` fails at 408/494 nodes: `scalar input 'prepare_transport_...Anthropic_Messages.max_tokens' has multiple upstream edges`. Root cause: lowerer creates ONE shared transport triplet per service operation, but `endpoint_use_count` resets per module — callables in different modules both wire literal sources to the same prepare node's scalar port. Fix: moved `endpoint_use_count` to global scope across all modules. Cross-module regression test added. | L1 | M | Done | BT-R3 |
 
 ### Postmortem: Testgen Discovery Bug (BT-R1)
 
@@ -470,18 +470,18 @@ Then run A1 → A2 → A3 and A4 → A5 once C20 is available.
 Delete every manual registry, extern bridge, and thin wrapper. Replace with DSL
 `data` declarations. After: adding a tool requires zero Rust changes.
 
-| # | IDs | What | Acceptance Criteria | Size |
-|---|-----|------|---------------------|------|
-| B1 | RT75 | **Gitignore patterns → DSL data.** `dsl/config/gitignore.dag` with 14 category `data` records. | Data section in `gitignore.rs` deleted. Generated `.gitignore` identical. | S |
-| B2 | RT80 | **Makegen registry → DSL data.** Move `BuildConfig`, `MetaTarget`, manual `ToolInfo` entries. Keep `ToolInfo::from_tool_def()` + Cargo commands (~400 lines). | `registry.rs` from 2,217 to ~400. Generated Makefile identical. | L |
-| B3 | RT74 | **Resource definitions → DSL data.** `dsl/config/resources.dag` with globs + output paths. | `resources.rs` deleted. Resource freshness works. | S |
-| B4 | RT76 | **Docgen targets → DSL data.** `dsl/tools/docgen.dag` data declaration. | `docgen/mod.rs` data deleted. Docgen reads from DSL. | S |
-| B5 | RT77 | **Delete `policy/pragma.rs`.** DSL rendering works (proven). Delete 546-line Rust mirror. | `pragma.rs` deleted. `make pragma` output identical. | S |
-| B6 | RT23 | **Delete `extern_impls.rs`.** Shadow bridges → DSL `extern func`. 2 recursive externs kept via inventory. | `extern_impls.rs` deleted. `lookup_extern_impl()` deleted. | M |
-| B7 | RT81 | **Delete tool wrappers.** 7 thin modules → generic `dsl_builder::build_dsl_graph_for_entrypoint()`. | `bootstrap/`, `build/`, `codegen/`, `infra/`, `gist.rs`, `deps_tool.rs` deleted. | S |
-| B8 | — | **Delete `embedded_assets.rs`.** Dead after extern deletion. | File deleted. | S |
-| B9 | — | **Delete compensating tests.** `tool_registration.rs`, `makefile_parity.rs`, `extern_ratchet.rs`. | 3 files deleted. `cargo test --workspace` passes. | S |
-| B10 | — | **Clean `makegen/shared.rs` + `justfile.rs`.** Remove deleted-registry references. | No references to deleted types. | S |
+| # | IDs | What | Acceptance Criteria | Size | Status |
+|---|-----|------|---------------------|------|--------|
+| B1 | RT75 | **Gitignore patterns → DSL data.** `dsl/config/gitignore.dag` with 14 category `data` records. | Data section in `gitignore.rs` deleted. Generated `.gitignore` identical. | S | Done |
+| B2 | RT80 | **Makegen registry → DSL data.** MetaTarget/CoreWorkflow data in DSL. Dead ToolInfo factory methods deleted. `registry.rs` reduced to 559 lines (from 672). `ToolInfo::from_tool_def()` + Cargo commands remain. Generated Makefile identical. | `registry.rs` lean. Generated Makefile identical. | L | Done |
+| B3 | RT74 | **Resource definitions → DSL data.** `dsl/config/resources.dag` with globs + output paths. | `resources.rs` deleted. Resource freshness works. | S | Done |
+| B4 | RT76 | **Docgen targets → DSL data.** `dsl/tools/docgen.dag` data declaration. | `docgen/mod.rs` data deleted. Docgen reads from DSL. | S | Done |
+| B5 | RT77 | **Delete `policy/pragma.rs`.** DSL rendering works (proven). Delete 546-line Rust mirror. | `pragma.rs` deleted. `make pragma` output identical. | S | Done |
+| B6 | RT23 | **Delete `extern_impls.rs`.** Shadow bridges → DSL `extern func`. 2 recursive externs kept via inventory. | `extern_impls.rs` deleted. `lookup_extern_impl()` deleted. | M | Done |
+| B7 | RT81 | **Delete tool wrappers.** 7 thin modules → generic `dsl_builder::build_dsl_graph_for_entrypoint()`. | `bootstrap/`, `build/`, `codegen/`, `infra/`, `gist.rs`, `deps_tool.rs` deleted. | S | Done |
+| B8 | — | **Delete `embedded_assets.rs`.** Dead after extern deletion. | File deleted. | S | Done |
+| B9 | — | **Delete compensating tests.** `tool_registration.rs`, `makefile_parity.rs`, `extern_ratchet.rs`. | 3 files deleted. `cargo test --workspace` passes. | S | Done |
+| B10 | — | **Clean `makegen/shared.rs` + `justfile.rs`.** Remove deleted-registry references. | No references to deleted types. | S | Done |
 
 **Chain**: B1 → B2 → B10; B3; B4; B5 → B6 → B7 → B8 → B9