Skip to content

Commit 8a38558

Browse files
authored
Merge pull request #10508 from Extra-Chill/cleanup/10299-dead-extension-manifest-surface
cleanup: delete dead extension-manifest surface
2 parents 6a95bd0 + 8c314be commit 8a38558

4 files changed

Lines changed: 39 additions & 61 deletions

File tree

crates/contracts/homeboy-extension-contract/src/manifest.rs

Lines changed: 28 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ pub struct ExtensionManifest {
165165
#[serde(skip_serializing_if = "Option::is_none")]
166166
pub requires: Option<RequirementsConfig>,
167167

168-
// Multi-extension composition: role ownership used to disambiguate a
168+
// Multi-extension composition: `includes` primacy is used to disambiguate a
169169
// capability provided by more than one linked extension.
170170
#[serde(default, skip_serializing_if = "Option::is_none")]
171171
pub composition: Option<CompositionConfig>,
@@ -179,33 +179,20 @@ pub struct ExtensionManifest {
179179
pub extension_path: Option<String>,
180180
}
181181

182-
/// Multi-extension composition metadata. Declares how a component's linked
183-
/// extensions relate so Homeboy can resolve a capability that more than one of
184-
/// them provides without requiring manual `capability_extensions` selection.
182+
/// Multi-extension composition metadata.
183+
///
184+
/// `includes` is the sole ownership signal: when several linked extensions
185+
/// provide the same capability, an extension that composes all the others is
186+
/// resolved as the primary owner. See
187+
/// `homeboy_core::extension_execution::disambiguate_capability_owner`.
188+
///
189+
/// Unknown keys are ignored rather than rejected, so manifests still carrying
190+
/// the retired `roles`/`optional`/`conflicts` metadata continue to load.
185191
#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)]
186192
pub struct CompositionConfig {
187-
/// Extensions this one composes with (informational).
193+
/// Extensions this one composes with. Used to resolve capability ownership.
188194
#[serde(default, skip_serializing_if = "Vec::is_empty")]
189195
pub includes: Vec<String>,
190-
/// Optional companion assets (informational).
191-
#[serde(default, skip_serializing_if = "Vec::is_empty")]
192-
pub optional: Vec<String>,
193-
/// Role name -> owning extension(s). A role with a single owner designates
194-
/// the extension that owns that role's capabilities across the composition.
195-
#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
196-
pub roles: BTreeMap<String, RoleOwners>,
197-
/// Extensions that must not be linked together (informational).
198-
#[serde(default, skip_serializing_if = "Vec::is_empty")]
199-
pub conflicts: Vec<String>,
200-
}
201-
202-
/// Owner(s) of a composition role. Manifests use both a single owner
203-
/// (`"javascript": "nodejs"`) and a list (`"project": ["a", "b"]`).
204-
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
205-
#[serde(untagged)]
206-
pub enum RoleOwners {
207-
One(String),
208-
Many(Vec<String>),
209196
}
210197

211198
impl ExtensionManifest {
@@ -574,7 +561,22 @@ mod composition_tests {
574561
use super::*;
575562

576563
#[test]
577-
fn deserializes_mixed_single_and_list_role_owners() {
564+
fn deserializes_includes() {
565+
let manifest: ExtensionManifest = serde_json::from_value(serde_json::json!({
566+
"name": "wordpress",
567+
"version": "1.0.0",
568+
"composition": { "includes": ["nodejs"] }
569+
}))
570+
.expect("manifest with composition deserializes");
571+
572+
let composition = manifest.composition.expect("composition present");
573+
assert_eq!(composition.includes, vec!["nodejs".to_string()]);
574+
}
575+
576+
/// Manifests published before `roles`/`optional`/`conflicts` were retired
577+
/// must still load — the keys are simply ignored rather than rejected.
578+
#[test]
579+
fn retired_composition_keys_are_tolerated() {
578580
let manifest: ExtensionManifest = serde_json::from_value(serde_json::json!({
579581
"name": "wordpress",
580582
"version": "1.0.0",
@@ -588,25 +590,10 @@ mod composition_tests {
588590
"conflicts": []
589591
}
590592
}))
591-
.expect("manifest with composition deserializes");
593+
.expect("manifest with retired composition keys still deserializes");
592594

593595
let composition = manifest.composition.expect("composition present");
594596
assert_eq!(composition.includes, vec!["nodejs".to_string()]);
595-
// A bare string owner deserializes into the single-owner shape.
596-
assert_eq!(
597-
composition.roles.get("javascript"),
598-
Some(&RoleOwners::One("nodejs".to_string()))
599-
);
600-
// A list owner deserializes into the multi-owner shape.
601-
assert_eq!(
602-
composition.roles.get("project"),
603-
Some(&RoleOwners::Many(vec![
604-
"wordpress-plugin".to_string(),
605-
"wordpress-theme".to_string(),
606-
]))
607-
);
608-
// Absent role.
609-
assert_eq!(composition.roles.get("missing"), None);
610597
}
611598

612599
#[test]

crates/contracts/homeboy-extension-contract/src/manifest_capability_config.rs

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,20 @@ pub struct ComponentEnvConfig {
3636
pub detect_script: String,
3737
}
3838

39-
/// What a extension provides: file extensions it handles and capabilities it supports.
39+
/// What an extension provides: file extensions it handles and how it is discovered.
40+
///
41+
/// Unknown keys are ignored rather than rejected. This struct previously used
42+
/// `deny_unknown_fields`, which made retiring any key a hard break: an older
43+
/// published manifest carrying a since-removed key would fail to deserialize
44+
/// entirely, taking `file_extensions` and `discovery_markers` down with it.
45+
/// Extensions ship on their own release cadence, so tolerance here matches the
46+
/// forward-compatibility policy the parent `ExtensionManifest` already sets
47+
/// with its `#[serde(flatten)] extra`.
4048
#[derive(Debug, Clone, Serialize, Deserialize)]
41-
#[serde(deny_unknown_fields)]
4249
pub struct ProvidesConfig {
4350
/// File extensions this extension can process.
4451
#[serde(default, skip_serializing_if = "Vec::is_empty")]
4552
pub file_extensions: Vec<String>,
46-
/// Capabilities this extension supports (e.g., ["fingerprint", "refactor"]).
47-
#[serde(default, skip_serializing_if = "Vec::is_empty")]
48-
pub capabilities: Vec<String>,
4953
/// Component-root marker rules used to suggest this extension for an
5054
/// unattached component. Core evaluates these generically; extension
5155
/// manifests own the ecosystem-specific file/glob knowledge.
@@ -80,12 +84,6 @@ pub struct ScriptsConfig {
8084
/// Receives `{file_path, content}` on stdin and outputs `{artifacts:[...]}` on stdout.
8185
#[serde(skip_serializing_if = "Option::is_none")]
8286
pub topology: Option<String>,
83-
/// Script that validates written code compiles/parses correctly.
84-
/// Receives `{root, changed_files}` JSON on stdin, exits 0 on success, non-zero with
85-
/// compiler output on stderr on failure.
86-
///
87-
#[serde(skip_serializing_if = "Option::is_none")]
88-
pub validate: Option<String>,
8987
/// Script that formats source code after automated writes.
9088
/// Runs from the project root. Exit 0 on success, non-zero on failure.
9189
/// Formatting failure is non-fatal — it logs a warning but never rolls back.
@@ -103,13 +101,6 @@ pub struct ScriptsConfig {
103101
/// Outputs `{fixes:[...]}` JSON using Homeboy's generic fix envelope.
104102
#[serde(skip_serializing_if = "Option::is_none")]
105103
pub compiler_warning_fixes: Option<String>,
106-
/// Script that extracts function contracts from source files.
107-
/// Receives `{file, content}` JSON on stdin, outputs `{file, contracts: [...]}` JSON on stdout.
108-
/// Each contract describes a function's signature, control flow branches, effects, and calls.
109-
///
110-
/// Used by the test generator, doc generator, and refactor safety checker.
111-
#[serde(skip_serializing_if = "Option::is_none")]
112-
pub contract: Option<String>,
113104
}
114105

115106
/// Extension-declared release preflight.

crates/homeboy-core/src/extension_execution.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ mod tests {
513513
std::fs::write(
514514
extension_dir.join(format!("{extension_id}.json")),
515515
format!(
516-
r#"{{"name":"{extension_id}","version":"1.0.0","{capability}":{{"extension_script":"{capability}.sh"}},"composition":{{"includes":[{includes_json}],"roles":{{"javascript":"nodejs","project":["a","b"]}}}}}}"#
516+
r#"{{"name":"{extension_id}","version":"1.0.0","{capability}":{{"extension_script":"{capability}.sh"}},"composition":{{"includes":[{includes_json}]}}}}"#
517517
),
518518
)
519519
.expect("extension manifest");

docs/internals/development/contracts/compiler-warning-extension.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Compiler Warning Extension Contract Gap
22

3-
Issue #2242 asks Homeboy core to move compiler-warning collection and compiler-warning fix generation behind extension-owned contracts. Validation and formatting already have extension script contracts (`scripts.validate` and `scripts.format`), but compiler warnings do not.
3+
Issue #2242 asks Homeboy core to move compiler-warning collection and compiler-warning fix generation behind extension-owned contracts. Formatting already has an extension script contract (`scripts.format`), but compiler warnings do not.
44

55
## Required Upstream Contract
66

0 commit comments

Comments
 (0)