Skip to content

Commit c46859d

Browse files
refactor: fold scaffolding-only crates and collapse duplicate enums (#10597)
* refactor(cli): collapse the duplicate command-family and stdout-mode enums CommandDispatchFamily was a byte-identical twin of CommandJsonFamily joined by an identity From impl, and CommandStdoutMode was a twin of CommandResponseMode differing only in the name of one variant. Both pairs encoded one concept under two types, forcing an identity conversion at every boundary. Delete both duplicates. The exported contract JSON is unchanged: json_family and dispatch_family were equal for every command by construction and still are, and response_mode/stdout_mode keep their distinct spellings (json vs json_envelope) by rendering the one enum through two functions. Two tests lock that wire shape. Also drops CommandSpec::dispatch_family and the never-called registered_command_dispatch_family, both of which became literal aliases. Refs #10295 * refactor: fold four scaffolding-only crates into their single consumers 43 workspace members -> 39. homeboy-output (714 LOC) and homeboy-process (1332 LOC) were leaf crates with exactly one and two dependents. Core re-exported both wholesale, so every call site already spelled them crate::output:: / crate::process:: and no import churn was needed. Neither is depended on by anything below core, so folding them in cannot reintroduce a cycle: homeboy-output has no internal dependencies at all, and homeboy-process depends only on homeboy-error, which core already depends on. homeboy-cli-contract (41 LOC: Placement) and homeboy-agents-contract (45 LOC: AgentTaskProviderRunnerSource) each held one type. Both move into homeboy-lab-runner-contract, which is already the below-core home for behavior-free runner contracts and is already a dependency of every consumer except homeboy-cli. Placement's stated rationale -- give core a place to name the type without a core -> commands edge -- holds there unchanged, and every value Placement takes is a statement about the Lab runner. Six process items that were only pub to cross the old crate boundary (SIGNAL_TERMINATE, SIGNAL_KILL, signal_pid, wait_for_pid_exit, pid_has_ownership_token, terminate_pid_with_sigterm_and_wait) narrow back to pub(crate); all six are consumed only by core's daemon module. Adds the first tests either folded contract type has had: Placement's operator-facing --placement value names and routing predicates, and the provider source-root wire shape (optional fields stay optional, unknown keys land in extra). Refs #10296 * refactor(process): drop dead terminate_process_tree_best_effort Zero callers across the workspace (verified with grep -rn plus a control grep). Superseded by the timeout-bounded owned-tree termination below it. * build: drop the folded crates from Cargo.lock CI passes --locked, so a stale lock fails every gate before compilation. Regenerated with cargo metadata (resolve only, no build): removes the four folded packages and repoints homeboy-lab-runner-contract. No version bumps. --------- Co-authored-by: chubes-bot <266378653+homeboy-ci[bot]@users.noreply.github.com>
1 parent 1736856 commit c46859d

40 files changed

Lines changed: 352 additions & 364 deletions

File tree

Cargo.lock

Lines changed: 3 additions & 42 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/contracts/homeboy-agents-contract/Cargo.toml

Lines changed: 0 additions & 15 deletions
This file was deleted.

crates/contracts/homeboy-agents-contract/src/lib.rs

Lines changed: 0 additions & 10 deletions
This file was deleted.

crates/contracts/homeboy-cli-contract/Cargo.toml

Lines changed: 0 additions & 17 deletions
This file was deleted.

crates/contracts/homeboy-cli-contract/src/lib.rs

Lines changed: 0 additions & 41 deletions
This file was deleted.

crates/contracts/homeboy-lab-runner-contract/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ version = "0.1.0"
44
edition = "2021"
55
license = "MIT"
66
authors = ["Chris Huber <chubes@extrachill.com>"]
7-
description = "Shared runner contract types + env-var constants for homeboy. Behavior-free data that both homeboy-core and the optional homeboy-runner feature crate depend on, so core can reference runner concepts without depending on runner behavior."
7+
description = "Shared runner contract types + env-var constants for homeboy, including the requested execution Placement and the managed provider source-root contract. Behavior-free data that both homeboy-core and the optional homeboy-runner feature crate depend on, so core can reference runner concepts without depending on runner behavior."
88
repository = "https://github.com/Extra-Chill/homeboy"
99
publish = false
1010

@@ -13,5 +13,6 @@ name = "homeboy_lab_runner_contract"
1313
path = "src/lib.rs"
1414

1515
[dependencies]
16+
clap = { version = "4.5", features = ["derive", "string"] }
1617
serde = { version = "1.0", features = ["derive"] }
1718
serde_json = "1.0"

crates/contracts/homeboy-lab-runner-contract/src/lib.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,22 @@
66
//! (e.g. the runner kind, or the env-var markers used when an exec crosses a
77
//! remote-runner boundary). Those plain-data contracts live here so core can
88
//! reference them without a `core -> runner` edge.
9+
//!
10+
//! Two single-type crates were folded in here for the same reason they existed
11+
//! separately — both are behavior-free runner contracts below core, and both
12+
//! were already depended on by a subset of this crate's dependents:
13+
//!
14+
//! - [`Placement`] (was `homeboy-cli-contract`): the requested execution
15+
//! location, read by core's Lab routing and by the runner's Lab selection.
16+
//! - [`AgentTaskProviderRunnerSource`] (was `homeboy-agents-contract`): a
17+
//! managed source checkout homeboy keeps synced on the runner, read by core's
18+
//! agent-runtime manifest and by `homeboy-agents`.
19+
20+
mod placement;
21+
mod provider_source_types;
22+
23+
pub use placement::Placement;
24+
pub use provider_source_types::AgentTaskProviderRunnerSource;
925

1026
use std::collections::{BTreeMap, BTreeSet};
1127

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
//! Requested execution placement.
2+
//!
3+
//! [`Placement`] sits at the boundary between the CLI argument surface and core
4+
//! Lab routing. It lives below core, in this crate rather than in the CLI, so
5+
//! `core` can name it without depending on the full `commands`/clap CLI
6+
//! definition (which would create a `core -> commands` dependency edge).
7+
//!
8+
//! It belongs with the rest of the runner contract because every value it takes
9+
//! is a statement about the Lab runner: whether to attempt an offload, and
10+
//! whether controller execution is an acceptable fallback.
11+
12+
use clap::ValueEnum;
13+
use serde::{Deserialize, Serialize};
14+
15+
/// The requested execution location. This is normalized once at the CLI
16+
/// boundary and is the only placement input used by routing code.
17+
#[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum, Serialize, Deserialize)]
18+
#[value(rename_all = "lower")]
19+
pub enum Placement {
20+
Auto,
21+
Local,
22+
Lab,
23+
#[value(name = "lab-or-local")]
24+
LabOrLocal,
25+
}
26+
27+
impl Default for Placement {
28+
fn default() -> Self {
29+
Self::Auto
30+
}
31+
}
32+
33+
impl Placement {
34+
/// Explicitly permit controller execution when an intended Lab offload
35+
/// cannot proceed. `Auto` retains the existing default routing behavior.
36+
pub const fn allows_local_fallback(self) -> bool {
37+
matches!(self, Self::LabOrLocal)
38+
}
39+
40+
/// Whether the operator requested a Lab attempt instead of leaving the
41+
/// command to its automatic routing policy.
42+
pub const fn requests_lab(self) -> bool {
43+
matches!(self, Self::Lab | Self::LabOrLocal)
44+
}
45+
}
46+
47+
#[cfg(test)]
48+
mod tests {
49+
use super::*;
50+
51+
/// `--placement <value>` is an operator-facing surface: the accepted
52+
/// spellings are part of the CLI contract, not an implementation detail of
53+
/// whichever crate happens to hold the enum. Asserted here so relocating
54+
/// the type cannot quietly rename a flag value.
55+
#[test]
56+
fn placement_keeps_its_operator_facing_value_names() {
57+
let names: Vec<String> = Placement::value_variants()
58+
.iter()
59+
.map(|variant| {
60+
variant
61+
.to_possible_value()
62+
.expect("every placement variant is selectable")
63+
.get_name()
64+
.to_string()
65+
})
66+
.collect();
67+
68+
assert_eq!(names, ["auto", "local", "lab", "lab-or-local"]);
69+
}
70+
71+
#[test]
72+
fn placement_defaults_to_auto_routing() {
73+
assert_eq!(Placement::default(), Placement::Auto);
74+
assert!(!Placement::default().requests_lab());
75+
assert!(!Placement::default().allows_local_fallback());
76+
}
77+
78+
/// Only `lab-or-local` both requests a Lab attempt and permits falling back
79+
/// to the controller; `lab` requests without permitting, and `local`/`auto`
80+
/// do neither.
81+
#[test]
82+
fn only_lab_or_local_both_requests_lab_and_permits_local_fallback() {
83+
let matrix = [
84+
(Placement::Auto, false, false),
85+
(Placement::Local, false, false),
86+
(Placement::Lab, true, false),
87+
(Placement::LabOrLocal, true, true),
88+
];
89+
90+
for (placement, requests_lab, allows_fallback) in matrix {
91+
assert_eq!(
92+
placement.requests_lab(),
93+
requests_lab,
94+
"{placement:?} requests_lab"
95+
);
96+
assert_eq!(
97+
placement.allows_local_fallback(),
98+
allows_fallback,
99+
"{placement:?} allows_local_fallback"
100+
);
101+
}
102+
}
103+
}

0 commit comments

Comments
 (0)