Skip to content

Commit 137bcc2

Browse files
Test Userclaude
andcommitted
fix(terraphim_rlm): stub fcctl_core private dep to allow --all-features compilation; add CI gate
The `firecracker` feature in `terraphim_rlm` imported `fcctl_core` (a private repo, commented out in Cargo.toml). With `--all-features` the feature was enabled and the module tried to compile — producing 10 errors. This is exactly the regression class issue #1295 wants to prevent. Fix A (Cargo.toml): - Declare `terraphim-firecracker` workspace crate as an optional dep, enabled by the `firecracker` feature (was `firecracker = []` with no deps). Fix B (firecracker.rs): - Replace the three `use fcctl_core::*` imports with compile-time stub types (`VmManager`, `SnapshotManager`, `SnapshotType`, `FcVmConfig`, `FcVmType`, `FcVmClient`, `FcSnapshotInfo`). Stubs carry the same API shape so the type checker accepts the existing code paths. Methods return errors; runtime behaviour is unchanged because `initialize()` returns `Err` before setting any `Option<VmManager>` / `Option<SnapshotManager>` (which stay `None` forever in stub mode, so `if let Some(ref vm)` arms are dead code). - Replace `fcctl_core::firecracker::VmConfig { ... VmType::Minimal }` calls with the new `FcVmConfig { ... FcVmType::Minimal }` stubs. - Fix `ip.to_string()` call (E0308: mismatched types). CI gate (ci-native.yml): - Add `check-all-features` job running `cargo check --workspace --all-features` after `lint-and-format`, sharing the sccache layer. - Scoped to `cargo check` (not test) to stay under 3 minutes. Closes #1295 Refs #2226 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 489b104 commit 137bcc2

4 files changed

Lines changed: 172 additions & 11 deletions

File tree

.github/workflows/ci-native.yml

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,68 @@ jobs:
167167
# rchd's queue, sharing slots with ADF agents.
168168
run: /home/alex/.local/bin/rch exec -- ./scripts/ci-check-format.sh
169169

170+
- name: sccache stats
171+
if: always()
172+
run: /home/alex/.local/bin/sccache --show-stats
173+
174+
check-all-features:
175+
runs-on: [self-hosted, bigbox]
176+
timeout-minutes: 15
177+
needs: [setup]
178+
# Verify that enabling all workspace features compiles cleanly.
179+
# Prevents the regression class identified in issue #1266 (and tracked by #1295)
180+
# where `cargo build` passes but `cargo build --all-features` silently fails
181+
# due to feature-gated type mismatches or missing optional dependencies.
182+
env:
183+
RUSTC_WRAPPER: /home/alex/.local/bin/sccache
184+
SCCACHE_BUCKET: rust-cache
185+
SCCACHE_SERVER_PORT: "4231"
186+
SCCACHE_ENDPOINT: http://172.26.0.1:8333
187+
SCCACHE_S3_USE_SSL: "false"
188+
SCCACHE_REGION: us-east-1
189+
SCCACHE_S3_KEY_PREFIX: terraphim-ai
190+
AWS_ACCESS_KEY_ID: any
191+
AWS_SECRET_ACCESS_KEY: any
192+
CARGO_INCREMENTAL: "0"
193+
steps:
194+
- name: Pre-checkout cleanup
195+
run: |
196+
WORKDIR="${GITHUB_WORKSPACE:-$PWD}"
197+
sudo rm -rf "${WORKDIR}/target" || true
198+
sudo rm -rf "${WORKDIR}/.cargo" || true
199+
200+
- name: Checkout code
201+
uses: actions/checkout@v6
202+
with:
203+
clean: true
204+
205+
- name: Install Rust
206+
uses: dtolnay/rust-toolchain@stable
207+
with:
208+
toolchain: 1.87.0
209+
210+
- name: Cache Cargo dependencies
211+
uses: actions/cache@v4
212+
with:
213+
path: |
214+
~/.cargo/registry
215+
~/.cargo/git
216+
target
217+
key: ${{ needs.setup.outputs.cache-key }}-cargo-all-features-${{ hashFiles('**/Cargo.lock') }}
218+
restore-keys: |
219+
${{ needs.setup.outputs.cache-key }}-cargo-all-features-
220+
221+
- name: sccache start and zero stats
222+
run: |
223+
/home/alex/.local/bin/sccache --start-server || true
224+
/home/alex/.local/bin/sccache --zero-stats
225+
226+
- name: Check workspace with all features
227+
# cargo check (not cargo test) keeps this under 3 minutes.
228+
# This gate catches feature-gated compilation regressions without
229+
# running the full test suite a second time.
230+
run: cargo check --workspace --all-features
231+
170232
- name: sccache stats
171233
if: always()
172234
run: /home/alex/.local/bin/sccache --show-stats

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/terraphim_rlm/Cargo.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ terraphim_automata = { path = "../terraphim_automata", optional = true }
3030
terraphim_types = { path = "../terraphim_types", optional = true }
3131
terraphim_rolegraph = { path = "../terraphim_rolegraph", optional = true }
3232
terraphim_agent_supervisor = { path = "../terraphim_agent_supervisor", optional = true }
33+
# Pool management for Firecracker VMs (workspace crate, CI-safe).
34+
# Note: package name uses a hyphen; Rust code uses underscores as the crate name.
35+
terraphim-firecracker = { path = "../../terraphim_firecracker", optional = true }
3336

3437
# Firecracker-rust core for VM and snapshot management (private repo).
3538
# Uncomment when building with --features firecracker on a machine with
@@ -72,7 +75,7 @@ kg-validation = ["dep:terraphim_automata", "dep:terraphim_types", "dep:terraphim
7275
supervision = ["dep:terraphim_agent_supervisor"]
7376
llm-bridge = ["dep:hyper", "dep:hyper-util"]
7477
docker-backend = ["dep:bollard"]
75-
firecracker = []
78+
firecracker = ["dep:terraphim-firecracker"]
7679
e2b-backend = ["dep:reqwest"]
7780
mcp = ["dep:rmcp"]
7881
cli = ["dep:clap", "dep:env_logger"]

crates/terraphim_rlm/src/executor/firecracker.rs

Lines changed: 105 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,110 @@ use std::path::PathBuf;
2323
use std::sync::Arc;
2424
use std::time::Instant;
2525

26-
// Use fcctl-core for VM and snapshot management
27-
// Note: SnapshotType must come from firecracker::models to match SnapshotManager's expected type
28-
use fcctl_core::firecracker::models::SnapshotType;
29-
use fcctl_core::snapshot::SnapshotManager;
30-
use fcctl_core::vm::VmManager;
31-
32-
// Keep terraphim_firecracker for pool management
26+
// Pool management from the workspace crate (CI-safe).
3327
use terraphim_firecracker::{PoolConfig, Sub2SecondOptimizer, VmPoolManager};
3428

29+
// Compile-time stubs replacing fcctl_core (private repo, not available on CI).
30+
// To use the real implementation:
31+
// 1. Uncomment fcctl-core in Cargo.toml [dependencies]
32+
// 2. Replace these stubs with: use fcctl_core::firecracker::models::SnapshotType;
33+
// use fcctl_core::snapshot::SnapshotManager;
34+
// use fcctl_core::vm::VmManager;
35+
36+
/// VM snapshot type. Stub for `fcctl_core::firecracker::models::SnapshotType`.
37+
#[allow(dead_code)]
38+
enum SnapshotType {
39+
Full,
40+
}
41+
42+
/// Opaque client returned by `VmManager::get_vm_client`.
43+
/// Stub for the real type from fcctl_core.
44+
#[allow(dead_code)]
45+
struct FcVmClient;
46+
47+
/// VM configuration passed to `VmManager::create_vm`.
48+
/// Stub for `fcctl_core::firecracker::VmConfig`.
49+
#[allow(dead_code)]
50+
struct FcVmConfig {
51+
kernel_path: String,
52+
rootfs_path: String,
53+
vcpus: u32,
54+
memory_mb: u32,
55+
initrd_path: Option<PathBuf>,
56+
boot_args: Option<String>,
57+
vm_type: FcVmType,
58+
}
59+
60+
/// VM type selector. Stub for `fcctl_core::firecracker::VmType`.
61+
#[allow(dead_code)]
62+
enum FcVmType {
63+
Minimal,
64+
}
65+
66+
/// VM lifecycle manager. Stub for `fcctl_core::vm::VmManager`.
67+
/// All methods return errors; real functionality requires the private crate.
68+
#[allow(dead_code)]
69+
struct VmManager;
70+
71+
#[allow(dead_code)]
72+
impl VmManager {
73+
fn new(_bin: &PathBuf, _socket: &PathBuf, _opts: Option<()>) -> Result<Self, String> {
74+
Err("fcctl_core not available — see Cargo.toml comments".to_string())
75+
}
76+
fn get_vm_ip(&self, _id: &str) -> Result<String, String> {
77+
Err("not available".to_string())
78+
}
79+
async fn get_vm_client(&mut self, _id: &str) -> Result<FcVmClient, String> {
80+
Err("not available".to_string())
81+
}
82+
async fn create_vm(&mut self, _config: &FcVmConfig, _opts: Option<()>) -> Result<String, String> {
83+
Err("not available".to_string())
84+
}
85+
}
86+
87+
/// Snapshot info entry returned by `SnapshotManager::list_snapshots`.
88+
#[allow(dead_code)]
89+
struct FcSnapshotInfo {
90+
pub id: String,
91+
}
92+
93+
/// Snapshot lifecycle manager. Stub for `fcctl_core::snapshot::SnapshotManager`.
94+
#[allow(dead_code)]
95+
struct SnapshotManager;
96+
97+
#[allow(dead_code)]
98+
impl SnapshotManager {
99+
fn new(_dir: &PathBuf, _opts: Option<()>) -> Result<Self, String> {
100+
Err("fcctl_core not available — see Cargo.toml comments".to_string())
101+
}
102+
async fn create_snapshot(
103+
&mut self,
104+
_client: FcVmClient,
105+
_vm_id: &str,
106+
_name: &str,
107+
_snap_type: SnapshotType,
108+
_opts: Option<()>,
109+
) -> Result<String, String> {
110+
Err("not available".to_string())
111+
}
112+
async fn restore_snapshot(
113+
&mut self,
114+
_client: FcVmClient,
115+
_snapshot_id: &str,
116+
) -> Result<(), String> {
117+
Err("not available".to_string())
118+
}
119+
async fn list_snapshots(
120+
&mut self,
121+
_filter: Option<&str>,
122+
) -> Result<Vec<FcSnapshotInfo>, String> {
123+
Ok(vec![])
124+
}
125+
async fn delete_snapshot(&mut self, _id: &str, _force: bool) -> Result<(), String> {
126+
Err("not available".to_string())
127+
}
128+
}
129+
35130
use super::ssh::SshExecutor;
36131
use super::{Capability, ExecutionContext, ExecutionResult, SnapshotId, ValidationResult};
37132
use crate::config::{BackendType, RlmConfig};
@@ -247,14 +342,14 @@ impl FirecrackerExecutor {
247342
"console=ttyS0 reboot=k panic=1 pci=off".to_string()
248343
};
249344

250-
let vm_config = fcctl_core::firecracker::VmConfig {
345+
let vm_config = FcVmConfig {
251346
kernel_path: "/tmp/vmlinux-5.10.225".to_string(),
252347
rootfs_path: "/tmp/ubuntu-22.04.ext4".to_string(),
253348
vcpus: 2,
254349
memory_mb: 1024,
255350
initrd_path: None,
256351
boot_args: Some(boot_args),
257-
vm_type: fcctl_core::firecracker::VmType::Minimal,
352+
vm_type: FcVmType::Minimal,
258353
};
259354

260355
match vm_manager.create_vm(&vm_config, None).await {
@@ -425,7 +520,7 @@ impl FirecrackerExecutor {
425520
match result {
426521
Ok(mut res) => {
427522
res.metadata.insert("vm_id".to_string(), id.clone());
428-
res.metadata.insert("vm_ip".to_string(), ip);
523+
res.metadata.insert("vm_ip".to_string(), ip.to_string());
429524
res.metadata
430525
.insert("backend".to_string(), "firecracker".to_string());
431526
Ok(res)

0 commit comments

Comments
 (0)