Skip to content

Fix #1295+#2226: stub fcctl_core private dep; add --all-features CI gate#906

Open
AlexMikhalev wants to merge 12 commits into
mainfrom
task/2226-fix-terraphim-rlm-all-features
Open

Fix #1295+#2226: stub fcctl_core private dep; add --all-features CI gate#906
AlexMikhalev wants to merge 12 commits into
mainfrom
task/2226-fix-terraphim-rlm-all-features

Conversation

@AlexMikhalev
Copy link
Copy Markdown
Contributor

Summary

Fixes the regression class identified in issue terraphim/terraphim-ai#1295 (Gitea).

  • Replaces three fcctl_core (private repo) imports in terraphim_rlm/src/executor/firecracker.rs with compile-time stub types so --all-features compiles cleanly without the private dep.
  • Declares terraphim-firecracker workspace crate as an optional dependency in terraphim_rlm/Cargo.toml (was missing).
  • Adds a check-all-features CI job to ci-native.yml running cargo check --workspace --all-features.

Verification

cargo check --workspace --all-features → PASS (was 10 errors before)
cargo clippy --workspace -- -D warnings → PASS
cargo test -p terraphim_rlm → PASS (7 doctests)

Refs terraphim/terraphim-ai#1295 (Gitea)
Refs terraphim/terraphim-ai#2226 (Gitea)

Test User and others added 12 commits June 4, 2026 00:14
…1992

reqwest::Error from error_for_status() on a 400 response is an HTTP
application error, not a transport/connectivity failure. Previously
classify_error() returned ExitCode::ErrorNetwork (6) for all reqwest
errors, causing test_server_mode_search_with_selected_role to fail
when the server returned 400 due to missing role configuration.

Now check re.status() before falling back to ErrorNetwork:
- 401 / 403  → ErrorAuth (5)
- 404        → ErrorNotFound (4)
- other 4xx/5xx → ErrorGeneral (1)
- no status  → ErrorNetwork (6, true connectivity failure)

Adds five regression tests using a real one-shot TCP server so no
mocks are needed, covering 400, 401, 403, 404, 500 paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds an integration test that starts a real TCP listener replying with
HTTP 400 Bad Request and asserts the binary exits 0 or 1 (never 6).
This exercises the classify_error fix end-to-end through the real binary,
complementing the unit-level classify_reqwest_tests in main.rs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…_node_ids Refs #2039

Deserialising a SerializableRoleGraph JSON written before issue #84 (trigger-based
KG retrieval) was merged would fail with a missing-field error because
trigger_descriptions and pinned_node_ids had no serde(default) annotation.

Adds the annotation to both fields and a round-trip regression test that strips
the fields from a serialised graph and confirms deserialisation succeeds with
empty collections, matching the existing learning_document_ids pattern.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…2133

Replace `for stream in incoming() { if let Ok(s) = stream { ... } }` with
`for s in incoming().flatten() { ... }` to satisfy clippy::manual_flatten.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(feature = "firecracker")

firecracker.rs called get_vm_status() and execute_vm_code() from ApiClient
which are #[cfg(feature = "firecracker")] but the module was compiled unconditionally.
This broke cargo test -p terraphim_agent with default features.

Fix: add #[cfg(feature = "firecracker")] to pub mod firecracker in modes/mod.rs
and update HybridExecutor to conditionally use FirecrackerExecutor only when
the feature is enabled, falling back to LocalExecutor otherwise.

Refs #2164

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- haystack_atlassian: add /// to ConfluenceClient, JiraClient, AtlassianClient
- haystack_core: add /// to HaystackProvider trait and associated items
- haystack_discourse: add /// to DiscourseClient and Post re-exports
- terraphim_ccusage: add /// to all public types and CcusageClient methods
- terraphim_kg_linter: add /// to all public structs, enums, and functions
- terraphim_negative_contribution: add /// to re-exported public items
- CHANGELOG: add firecracker gate, clippy manual_flatten, rolegraph serde
  defaults, and rustdoc coverage improvement entries

Workspace doc coverage: 81% (was 30% at prior scan). Zero-coverage crates: 1.

Refs #2136
…ubmodules

- haystack_discourse: add /// to DiscourseClient struct, new(), Post struct
- terraphim_negative_contribution: add /// to NegativeContributionScanner
  and all pub methods (new, from_thesaurus, scan_file, scan_files,
  scan_to_output, thesaurus)

Reduces zero-coverage crate count to 0.

Refs #2136
…es 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant