From e49d0a8f036ed35e43ce1c687f62cd653f0874b5 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Tue, 18 Aug 2026 09:57:46 -0700 Subject: [PATCH 01/65] fix(boundary): accept canonical consumer inventory schema --- .../src/inventory/dependency_policy.rs | 47 +++- crates/sc-lint-boundary/src/inventory/mod.rs | 50 +++-- .../sc-lint-boundary/src/inventory/tests.rs | 210 ++++++++++++++++++ .../sc-lint-boundary/src/inventory/types.rs | 39 +++- crates/sc-lint/src/tests.rs | 6 +- 5 files changed, 326 insertions(+), 26 deletions(-) diff --git a/crates/sc-lint-boundary/src/inventory/dependency_policy.rs b/crates/sc-lint-boundary/src/inventory/dependency_policy.rs index 4be5a8ba..108a6208 100644 --- a/crates/sc-lint-boundary/src/inventory/dependency_policy.rs +++ b/crates/sc-lint-boundary/src/inventory/dependency_policy.rs @@ -9,10 +9,10 @@ use super::types::BoundaryRecord; use super::types::RawBoundaryRecord; #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -#[serde(deny_unknown_fields)] -pub(crate) struct RawForbiddenPackageEdge { - pub(crate) from: String, - pub(crate) to: String, +#[serde(untagged)] +pub(crate) enum RawForbiddenPackageEdge { + Structured { from: String, to: String }, + ArrowDelimited(String), } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -83,6 +83,11 @@ pub(crate) enum DependencyPolicyError { field: &'static str, package: WorkspacePackageName, }, + #[error("invalid forbidden edge {value:?} in boundary `{boundary_id}`: expected `from -> to`")] + InvalidForbiddenEdge { + boundary_id: BoundaryId, + value: String, + }, } impl RawDependenciesSection { @@ -101,10 +106,10 @@ impl RawDependenciesSection { let mut forbidden_edges = Vec::with_capacity(self.forbidden_edges.len()); let mut seen_edges = BTreeSet::new(); for raw_edge in self.forbidden_edges { + let (raw_from, raw_to) = raw_edge.into_parts(boundary_id)?; let from = - parse_workspace_package_name(raw_edge.from, boundary_id, "forbidden_edges[].from")?; - let to = - parse_workspace_package_name(raw_edge.to, boundary_id, "forbidden_edges[].to")?; + parse_workspace_package_name(raw_from, boundary_id, "forbidden_edges[].from")?; + let to = parse_workspace_package_name(raw_to, boundary_id, "forbidden_edges[].to")?; let edge = ForbiddenPackageEdge { from: from.clone(), to: to.clone(), @@ -127,6 +132,32 @@ impl RawDependenciesSection { } } +impl RawForbiddenPackageEdge { + fn into_parts( + self, + boundary_id: &BoundaryId, + ) -> std::result::Result<(String, String), DependencyPolicyError> { + match self { + Self::Structured { from, to } => Ok((from, to)), + Self::ArrowDelimited(value) => { + let Some((from, to)) = value.split_once("->") else { + return Err(DependencyPolicyError::InvalidForbiddenEdge { + boundary_id: boundary_id.clone(), + value, + }); + }; + if from.contains("->") || to.contains("->") { + return Err(DependencyPolicyError::InvalidForbiddenEdge { + boundary_id: boundary_id.clone(), + value, + }); + } + Ok((from.trim().to_string(), to.trim().to_string())) + } + } + } +} + fn validate_package_list( boundary_id: &BoundaryId, field: &'static str, @@ -175,8 +206,10 @@ impl TryFrom for BoundaryRecord { public: value.public, implementation: value.implementation, composition: value.composition, + ownership: value.ownership, callers: value.callers, references: value.references, + contracts: value.contracts, testing: value.testing, enforcement: value.enforcement, status: value.status, diff --git a/crates/sc-lint-boundary/src/inventory/mod.rs b/crates/sc-lint-boundary/src/inventory/mod.rs index be6145ce..cfb37f82 100644 --- a/crates/sc-lint-boundary/src/inventory/mod.rs +++ b/crates/sc-lint-boundary/src/inventory/mod.rs @@ -59,8 +59,18 @@ pub(crate) fn load_boundary_inventory(root: &Path) -> Result } let planning_path = boundaries_root.join("planning.toml"); - let planning: types::PlanningMetadata = parse_toml_file(&planning_path)?; - validate_planning_metadata(&planning, &planning_path)?; + let planning = if planning_path.exists() { + let planning: types::PlanningMetadata = parse_toml_file(&planning_path)?; + validate_planning_metadata(&planning, &planning_path)?; + planning + } else { + types::PlanningMetadata { + planning: types::PlanningHeader { + current_sprint: types::SprintId::placeholder_empty_inventory(), + }, + planned_items: BTreeMap::new(), + } + }; Ok(BoundaryInventory { records, planning }) } @@ -128,16 +138,6 @@ fn validate_boundary_path( ); } - let expected_owner_crate_path = record.owner_package.replace('-', "_"); - if record.owner_crate_path.as_str() != expected_owner_crate_path { - anyhow::bail!( - "boundary `{}` declares owner_crate_path `{}` but expected `{expected_owner_crate_path}` from owner_package `{}`", - record.boundary_id, - record.owner_crate_path, - record.owner_package - ); - } - let relative = path.strip_prefix(boundaries_root).with_context(|| { format!( "boundary file `{}` is outside boundaries root", @@ -155,16 +155,36 @@ fn validate_boundary_path( } fn validate_boundary_schema(record: &BoundaryRecord, path: &Path) -> Result<()> { - if record.public.facade.trim().is_empty() { + let has_public_surface = [ + ("public.facade", record.public.facade.as_deref()), + ("public.trait", record.public.trait_name.as_deref()), + ] + .into_iter() + .map(|(field, value)| { + let value = value.map(str::trim); + if value.is_some_and(str::is_empty) { + anyhow::bail!( + "boundary `{}` in `{}` defines an empty {field}", + record.boundary_id, + path.display() + ); + } + Ok(value.is_some()) + }) + .collect::>>()? + .into_iter() + .any(|present| present); + + if !has_public_surface { anyhow::bail!( - "boundary `{}` in `{}` must define a non-empty public.facade", + "boundary `{}` in `{}` must define a non-empty public.facade or public.trait", record.boundary_id, path.display() ); } match record.implementation.visibility { - types::Visibility::Public => { + types::Visibility::Public | types::Visibility::Private | types::Visibility::PubCrate => { if record .implementation .implementation_type diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index d7b04712..6ad65073 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -99,6 +99,216 @@ fn loads_valid_boundary_inventory() { assert_eq!(inventory.planning.planning.current_sprint, "A.6"); } +#[test] +fn loads_trait_public_boundary_inventory() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.write( + "boundaries/sc-lint-directives/trait-boundary.toml", + r#" +boundary_id = "BOUNDARY-DirectiveTraitSurface" +owner_package = "sc-lint-directives" +owner_crate_path = "sc_lint_directives" +name = "DirectiveTraitSurface" + +[public] +trait = "Directive" +notes = "Trait surfaces are valid public boundary declarations." + +[implementation] +visibility = "trait_only" + +[composition] +roots = ["Directive"] + +[dependencies] +allowed_dependents = [] +allowed_dependencies = [] +forbidden_edges = [] + +[references] +scope = "outside_owner_crate" +forbidden = [] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = [] + +[status] +state = "concrete_landed" +"#, + ); + + let inventory = load_boundary_inventory(fixture.root()).expect("trait inventory loads"); + assert_eq!(inventory.records.len(), 2); + assert_eq!( + inventory.records[1].public.trait_name.as_deref(), + Some("Directive") + ); + assert_eq!(inventory.records[1].public.facade, None); +} + +#[test] +fn loads_boundary_inventory_without_planning_metadata() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fs::remove_file(fixture.root().join("boundaries/planning.toml")).expect("remove planning"); + + let inventory = + load_boundary_inventory(fixture.root()).expect("inventory loads without planning"); + + assert_eq!(inventory.planning.planning.current_sprint, "A.0"); + assert!(inventory.planning.planned_items.is_empty()); +} + +#[test] +fn loads_atm_boundary_vocabulary() { + let fixture = InventoryFixture::new(); + fixture.write( + "boundaries/atm/adapter.toml", + r#" +boundary_id = "BOUNDARY-AtmAdapter" +owner_package = "atm" +owner_crate_path = "atm" +name = "AtmAdapter" + +[public] +trait = "AdapterPort" +notes = "A public trait surface." + +[implementation] +type = "Adapter" +module = "atm::adapter" +visibility = "public" +constructor = "public" + +[composition] +roots = ["atm::bootstrap"] + +[ownership] +io_owns = ["adapter_io"] +io_forbidden = ["storage_io"] + +[dependencies] +allowed_dependents = [] +allowed_dependencies = ["atm-core"] +forbidden_edges = ["atm -> atm-storage"] + +[references] +scope = "global" +forbidden = [] + +[contracts] +request_types = ["Request"] +response_types = ["Response"] +error_types = ["AtmError"] +notes = ["Documented contract."] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = [] + +[status] +state = "active" +notes = ["Live."] +"#, + ); + fixture.write( + "boundaries/atm-private/private-adapter.toml", + r#" +boundary_id = "BOUNDARY-AtmPrivateAdapter" +owner_package = "atm-private" +owner_crate_path = "atm_private" +name = "AtmPrivateAdapter" + +[public] +facade = "private_adapter" + +[implementation] +type = "PrivateAdapter" +module = "atm_private::adapter" +visibility = "private" +constructor = "private" + +[composition] +roots = [] + +[dependencies] +allowed_dependents = [] +allowed_dependencies = [] +forbidden_edges = [] + +[references] +scope = "inside_owner_crate" +forbidden = [] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = [] + +[status] +state = "retired" +"#, + ); + fixture.write( + "boundaries/atm-crate/crate-adapter.toml", + r#" +boundary_id = "BOUNDARY-AtmCrateAdapter" +owner_package = "atm-crate" +owner_crate_path = "atm_crate" +name = "AtmCrateAdapter" + +[public] +facade = "crate_adapter" + +[implementation] +type = "CrateAdapter" +module = "atm_crate::adapter" +visibility = "pub(crate)" +constructor = "pub(crate)" + +[composition] +roots = [] + +[dependencies] +allowed_dependents = [] +allowed_dependencies = [] +forbidden_edges = [] + +[references] +scope = "outside_owner_crate" +forbidden = [] + +[testing] +allowed_test_double_paths = [] +forbidden_test_bypasses = [] + +[enforcement] +lint_rules = [] +review_gates = [] + +[status] +state = "unix_implemented_windows_pending" +"#, + ); + + let inventory = load_boundary_inventory(fixture.root()).expect("ATM vocabulary loads"); + + assert_eq!(inventory.records.len(), 3); +} + #[test] fn allows_trait_only_records_to_omit_type_and_module() { let fixture = InventoryFixture::new(); diff --git a/crates/sc-lint-boundary/src/inventory/types.rs b/crates/sc-lint-boundary/src/inventory/types.rs index 79e95847..a1c92d0e 100644 --- a/crates/sc-lint-boundary/src/inventory/types.rs +++ b/crates/sc-lint-boundary/src/inventory/types.rs @@ -432,9 +432,11 @@ pub(crate) struct RawBoundaryRecord { pub(crate) public: PublicSection, pub(crate) implementation: ImplementationSection, pub(crate) composition: CompositionSection, + pub(crate) ownership: Option, pub(crate) callers: Option, pub(crate) dependencies: RawDependenciesSection, pub(crate) references: ReferencesSection, + pub(crate) contracts: Option, pub(crate) testing: TestingSection, pub(crate) enforcement: EnforcementSection, pub(crate) status: StatusSection, @@ -449,9 +451,11 @@ pub(crate) struct BoundaryRecord { pub(crate) public: PublicSection, pub(crate) implementation: ImplementationSection, pub(crate) composition: CompositionSection, + pub(crate) ownership: Option, pub(crate) callers: Option, pub(crate) dependencies: PackageDependencyPolicy, pub(crate) references: ReferencesSection, + pub(crate) contracts: Option, pub(crate) testing: TestingSection, pub(crate) enforcement: EnforcementSection, pub(crate) status: StatusSection, @@ -460,7 +464,10 @@ pub(crate) struct BoundaryRecord { #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct PublicSection { - pub(crate) facade: String, + pub(crate) facade: Option, + #[serde(rename = "trait")] + pub(crate) trait_name: Option, + pub(crate) notes: Option, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -479,6 +486,13 @@ pub(crate) struct CompositionSection { pub(crate) roots: Vec, } +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct OwnershipSection { + pub(crate) io_owns: Vec, + pub(crate) io_forbidden: Vec, +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct CallersSection { @@ -499,6 +513,15 @@ pub(crate) struct ReferencesSection { pub(crate) forbidden: Vec, } +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct ContractsSection { + pub(crate) request_types: Vec, + pub(crate) response_types: Vec, + pub(crate) error_types: Vec, + pub(crate) notes: Option>, +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct TestingSection { @@ -517,6 +540,7 @@ pub(crate) struct EnforcementSection { #[serde(deny_unknown_fields)] pub(crate) struct StatusSection { pub(crate) state: BoundaryState, + pub(crate) notes: Option>, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] @@ -546,25 +570,38 @@ pub(crate) struct PlannedItem { pub(crate) enum Visibility { Public, TraitOnly, + Private, + #[serde(rename = "pub(crate)")] + PubCrate, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "snake_case")] pub(crate) enum Constructor { None, + Public, + Private, + #[serde(rename = "pub(crate)")] + PubCrate, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "snake_case")] pub(crate) enum ReferenceScope { + InsideOwnerCrate, OutsideOwnerCrate, + Global, } #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(rename_all = "snake_case")] pub(crate) enum BoundaryState { + Active, + Retired, Planned, ConcreteLanded, + StubLanded, + UnixImplementedWindowsPending, ReservedFuture, } diff --git a/crates/sc-lint/src/tests.rs b/crates/sc-lint/src/tests.rs index 9d41c532..e67731c0 100644 --- a/crates/sc-lint/src/tests.rs +++ b/crates/sc-lint/src/tests.rs @@ -1331,7 +1331,7 @@ fn malformed_backend_json_maps_to_backend_protocol_error() { } #[test] -fn backend_execution_failure_maps_to_backend_failure_error() { +fn empty_boundary_inventory_maps_to_backend_failure_error() { let temp_dir = TempDir::new().expect("temp dir"); std::fs::write( temp_dir.path().join("Cargo.toml"), @@ -1352,8 +1352,8 @@ fn backend_execution_failure_maps_to_backend_failure_error() { let loaded = LoadedConfig::load(&cli, &context).expect("config loads"); let error = crate::command::execute(&context, &loaded).expect_err("dispatch should fail"); - assert_eq!(error.kind, CliErrorKind::Config); - assert_eq!(error.code(), "CLI.CONFIG_ERROR"); + assert_eq!(error.kind, CliErrorKind::BackendFailure); + assert_eq!(error.code(), "CLI.BACKEND_EXEC_FAILURE"); assert!(error.cause.is_some()); assert!(std::error::Error::source(&error).is_some()); } From 36e6f6082f4e4d5f815a759a49c38ee14f624119 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Tue, 8 Sep 2026 17:52:01 -0700 Subject: [PATCH 02/65] chore: sync .atm.toml panes to herdr-only roster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes the stale tmux_pane_id from clint's pane and adds the flint, publisher, and spare panes so .atm.toml's launch config matches the current herdr-backend ATM roster. No post_send_hooks entries — nudging goes through the ATM/herdr backend, not .atm.toml. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_015iePmWRzj2yEmdVkGKdPcb --- .atm.toml | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/.atm.toml b/.atm.toml index e0e6f534..6a07c22e 100644 --- a/.atm.toml +++ b/.atm.toml @@ -5,6 +5,7 @@ default_team = "sc-lint" [rmux] session = "sc-lint" +# Window 1: Core agents [[rmux.windows]] name = "agents" layout = "even-horizontal" @@ -16,19 +17,42 @@ env = { ATM_IDENTITY = "team-lead", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ [[rmux.windows.panes]] name = "clint" -tmux_pane_id = "%46" command = "codex -c features.codex_hooks=true --yolo" env = { ATM_IDENTITY = "clint", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } +[[rmux.windows.panes]] +name = "quality-mgr" +model = "sonnet" +env = { ATM_IDENTITY = "quality-mgr", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } + +# Window 2: Specialists +[[rmux.windows]] +name = "support" +layout = "even-horizontal" + [[rmux.windows.panes]] name = "cfast" command = "codex -c features.codex_hooks=true --yolo" env = { ATM_IDENTITY = "cfast", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } [[rmux.windows.panes]] -name = "quality-mgr" +name = "flint" +model = "fable" +env = { ATM_IDENTITY = "flint", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } + +# Window 3: Publishing +[[rmux.windows]] +name = "publishing" +layout = "even-horizontal" + +[[rmux.windows.panes]] +name = "publisher" model = "sonnet" -env = { ATM_IDENTITY = "quality-mgr", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } +env = { ATM_IDENTITY = "publisher", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } + +[[rmux.windows.panes]] +name = "spare" +env = { ATM_TEAM = "sc-lint" } [startup.team-lead] all = ["Read /codex-orchestration SKILL.md"] From 37d51d4c5deb2ed02b3b95838a257c1b122516b0 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sun, 13 Sep 2026 14:30:42 -0700 Subject: [PATCH 03/65] chore(atm): replace deprecated features.codex_hooks with features.hooks --- .atm.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.atm.toml b/.atm.toml index e0e6f534..23a762d6 100644 --- a/.atm.toml +++ b/.atm.toml @@ -17,12 +17,12 @@ env = { ATM_IDENTITY = "team-lead", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ [[rmux.windows.panes]] name = "clint" tmux_pane_id = "%46" -command = "codex -c features.codex_hooks=true --yolo" +command = "codex -c features.hooks=true --yolo" env = { ATM_IDENTITY = "clint", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } [[rmux.windows.panes]] name = "cfast" -command = "codex -c features.codex_hooks=true --yolo" +command = "codex -c features.hooks=true --yolo" env = { ATM_IDENTITY = "cfast", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } [[rmux.windows.panes]] From 6351a3ea47d7cdb0956fd98bc0ad6f57acbdc6fe Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Tue, 15 Sep 2026 17:41:11 -0700 Subject: [PATCH 04/65] chore: backfill ATM roster aliases in .atm.toml Add alias fields (sc-lint-lead, quality-mgr-lint) matching the live ATM roster DB for team sc-lint. Co-Authored-By: Claude Sonnet 5 --- .atm.toml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.atm.toml b/.atm.toml index e0e6f534..eab0c014 100644 --- a/.atm.toml +++ b/.atm.toml @@ -11,6 +11,7 @@ layout = "even-horizontal" [[rmux.windows.panes]] name = "team-lead" +alias = "sc-lint-lead" model = "sonnet" env = { ATM_IDENTITY = "team-lead", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } @@ -27,6 +28,7 @@ env = { ATM_IDENTITY = "cfast", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = [[rmux.windows.panes]] name = "quality-mgr" +alias = "quality-mgr-lint" model = "sonnet" env = { ATM_IDENTITY = "quality-mgr", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } From 3388110ec1011316321f89341f9df822311866ca Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 10:43:06 -0700 Subject: [PATCH 05/65] chore(atm): drop stale tmux_pane_id and add publisher alias --- .atm.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.atm.toml b/.atm.toml index 858c1962..68af47d0 100644 --- a/.atm.toml +++ b/.atm.toml @@ -18,7 +18,6 @@ env = { ATM_IDENTITY = "team-lead", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ [[rmux.windows.panes]] name = "clint" -tmux_pane_id = "%46" command = "codex -c features.hooks=true --yolo" env = { ATM_IDENTITY = "clint", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } @@ -50,6 +49,7 @@ layout = "even-horizontal" [[rmux.windows.panes]] name = "publisher" +alias = "sc-lint-publisher" model = "sonnet" env = { ATM_IDENTITY = "publisher", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } From 73620b3c6b88d7f3aba531b648f2d12da0cf73b9 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 11:06:53 -0700 Subject: [PATCH 06/65] chore: integrate beads issue tracking --- .agents/skills/beads/SKILL.md | 80 ++++++++++++++++++++++++ .agents/skills/beads/agents/openai.yaml | 4 ++ .beads/.gitignore | 81 +++++++++++++++++++++++++ .beads/README.md | 81 +++++++++++++++++++++++++ .beads/config.yaml | 76 +++++++++++++++++++++++ .beads/hooks/post-checkout | 59 ++++++++++++++++++ .beads/hooks/post-merge | 59 ++++++++++++++++++ .beads/hooks/pre-commit | 59 ++++++++++++++++++ .beads/hooks/pre-push | 59 ++++++++++++++++++ .beads/hooks/prepare-commit-msg | 59 ++++++++++++++++++ .beads/metadata.json | 10 +++ .claude/settings.json | 15 +++++ .codex/config.toml | 2 + .codex/hooks.json | 51 ++++++++++++++++ .cursor/hooks.json | 20 ++++++ .cursor/rules/beads.mdc | 67 ++++++++++++++++++++ .gitignore | 7 +++ AGENTS.md | 79 ++++++++++++++++++++++++ CLAUDE.md | 56 +++++++++++++++++ 19 files changed, 924 insertions(+) create mode 100644 .agents/skills/beads/SKILL.md create mode 100644 .agents/skills/beads/agents/openai.yaml create mode 100644 .beads/.gitignore create mode 100644 .beads/README.md create mode 100644 .beads/config.yaml create mode 100755 .beads/hooks/post-checkout create mode 100755 .beads/hooks/post-merge create mode 100755 .beads/hooks/pre-commit create mode 100755 .beads/hooks/pre-push create mode 100755 .beads/hooks/prepare-commit-msg create mode 100644 .beads/metadata.json create mode 100644 .claude/settings.json create mode 100644 .codex/config.toml create mode 100644 .codex/hooks.json create mode 100644 .cursor/hooks.json create mode 100644 .cursor/rules/beads.mdc diff --git a/.agents/skills/beads/SKILL.md b/.agents/skills/beads/SKILL.md new file mode 100644 index 00000000..a5a3344c --- /dev/null +++ b/.agents/skills/beads/SKILL.md @@ -0,0 +1,80 @@ +--- +name: beads +description: Use when working in a repository that uses bd or Beads for durable project task tracking, issue dependencies, blocker management, multi-session handoff, or shared work memory. Trigger when the user asks to find ready work, claim or close tasks, create follow-up work, inspect blockers, recover project context, or choose between local planning and persistent project tracking. +--- + +# Beads + +Use Beads as the shared project task system. Local plans, scratch files, and personal memories are useful, but they are not the durable source of truth for project work. + +## First Step + +Run: + +```bash +bd prime +``` + +If that prints nothing, check whether the repository has an active Beads workspace: + +```bash +bd where +``` + +## Preferred Route + +Use the `bd` CLI when shell access is available. It is the most compact and direct Beads interface. + +## Core CLI Workflow + +1. Find work: + +```bash +bd ready +bd list --status=open +bd list --status=in_progress +``` + +2. Inspect before editing: + +```bash +bd show +``` + +3. Claim work atomically: + +```bash +bd update --claim +``` + +4. Create durable follow-up work when implementation reveals new tasks: + +```bash +bd create "Short title" --description="Why this exists and what needs to be done" --type=task --priority=2 +``` + +5. Close completed work: + +```bash +bd close --reason="Completed" +``` + +## What Belongs In Beads + +Use Beads for: + +- shared project tasks +- blockers and dependencies +- discovered follow-up work +- work that must survive thread reset, compaction, or handoff +- status that another person or agent should be able to resume + +Use agent-local planning tools only for the current turn's execution checklist. Do not treat them as shared project state. + +## Rules + +- Do not create markdown TODO files as the source of truth when Beads is available. +- Do not use `bd edit`; it opens an interactive editor. Use `bd update` flags instead. +- Prefer `--json` when parsing `bd` output programmatically. +- If hooks are installed, `bd prime` may already be injected. Run it manually when context is missing. +- Do not auto-close or mutate tasks unless the work is actually complete. diff --git a/.agents/skills/beads/agents/openai.yaml b/.agents/skills/beads/agents/openai.yaml new file mode 100644 index 00000000..09c3b8f6 --- /dev/null +++ b/.agents/skills/beads/agents/openai.yaml @@ -0,0 +1,4 @@ +interface: + display_name: "Beads" + short_description: "Project task tracking with bd" + default_prompt: "Use $beads to inspect ready work and manage durable project tasks." diff --git a/.beads/.gitignore b/.beads/.gitignore new file mode 100644 index 00000000..7ba2a936 --- /dev/null +++ b/.beads/.gitignore @@ -0,0 +1,81 @@ +# Dolt database (managed by Dolt, not git) +dolt/ +embeddeddolt/ +proxieddb/ + +# Runtime files +bd.sock +bd.sock.startlock +sync-state.json +last-touched +.exclusive-lock + +# Daemon runtime (lock, log, pid) +daemon.* + +# Push state (runtime, per-machine) +push-state.json + +# Lock files (various runtime locks) +*.lock + +# Credential key (encryption key for federation peer auth — never commit) +.beads-credential-key + +# Local version tracking (prevents upgrade notification spam after git ops) +.local_version + +proxied_server_client_info.json + +# Worktree redirect file (contains relative path to main repo's .beads/) +# Must not be committed as paths would be wrong in other clones +redirect + +# Sync state (local-only, per-machine) +# These files are machine-specific and should not be shared across clones +.sync.lock + +# Workspace operation gate (internal/workspacegate): physical-root gate +# files live beside the guarded root inside .beads (e.g. dolt.gate.lock) +*.gate.lock* +export-state/ +export-state.json +last_pull + +# Ephemeral store (SQLite - wisps/molecules, intentionally not versioned) +ephemeral.sqlite3 +ephemeral.sqlite3-journal +ephemeral.sqlite3-wal +ephemeral.sqlite3-shm + +# Dolt server management (auto-started by bd) +dolt-server.pid +dolt-server.log +dolt-server.lock +dolt-server.port +dolt-server.activity + +# Debug-mode pprof artifacts (written when dolt.debug: true in config.yaml) +dolt-pprof/ + +# Corrupt backup directories (created by bd doctor --fix recovery) +*.corrupt.backup/ + +# Backup data (auto-exported JSONL, local-only) +backup/ + +# Per-project environment file (Dolt connection config, GH#2520) +.env + +# Legacy files (from pre-Dolt versions) +*.db +*.db?* +*.db-journal +*.db-wal +*.db-shm +db.sqlite +bd.db +# NOTE: Do NOT add negation patterns here. +# They would override fork protection in .git/info/exclude. +# Config files (metadata.json, config.yaml) are tracked by git by default +# since no pattern above ignores them. diff --git a/.beads/README.md b/.beads/README.md new file mode 100644 index 00000000..63e8f4c2 --- /dev/null +++ b/.beads/README.md @@ -0,0 +1,81 @@ +# Beads - AI-Native Issue Tracking + +Welcome to Beads! This repository uses **Beads** for issue tracking - a modern, AI-native tool designed to live directly in your codebase alongside your code. + +## What is Beads? + +Beads is issue tracking that lives in your repo, making it perfect for AI coding agents and developers who want their issues close to their code. No web UI required - everything works through the CLI and integrates seamlessly with git. + +**Learn more:** [github.com/steveyegge/beads](https://github.com/steveyegge/beads) + +## Quick Start + +### Essential Commands + +```bash +# Create new issues +bd create "Add user authentication" + +# View all issues +bd list + +# View issue details +bd show + +# Update issue status +bd update --claim +bd update --status done + +# Sync with Dolt remote +bd dolt push +``` + +### Working with Issues + +Issues in Beads are: +- **Git-native**: Stored in Dolt database with version control and branching +- **AI-friendly**: CLI-first design works perfectly with AI coding agents +- **Branch-aware**: Issues can follow your branch workflow +- **Sync-ready**: Uses Dolt remotes for backup and team sharing + +## Why Beads? + +✨ **AI-Native Design** +- Built specifically for AI-assisted development workflows +- CLI-first interface works seamlessly with AI coding agents +- No context switching to web UIs + +🚀 **Developer Focused** +- Issues live in your repo, right next to your code +- Works offline, syncs when you push +- Fast, lightweight, and stays out of your way + +🔧 **Git Integration** +- Dolt-native sync via bd dolt push / bd dolt pull +- Branch-aware issue tracking +- Dolt-native three-way merge resolution + +## Get Started with Beads + +Try Beads in your own projects: + +```bash +# Install Beads +curl -sSL https://raw.githubusercontent.com/steveyegge/beads/main/scripts/install.sh | bash + +# Initialize in your repo +bd init + +# Create your first issue +bd create "Try out Beads" +``` + +## Learn More + +- **Documentation**: [github.com/steveyegge/beads/docs](https://github.com/steveyegge/beads/tree/main/docs) +- **Quick Start Guide**: Run `bd quickstart` +- **Examples**: [github.com/steveyegge/beads/examples](https://github.com/steveyegge/beads/tree/main/examples) + +--- + +*Beads: Issue tracking that moves at the speed of thought* ⚡ diff --git a/.beads/config.yaml b/.beads/config.yaml new file mode 100644 index 00000000..b5158af1 --- /dev/null +++ b/.beads/config.yaml @@ -0,0 +1,76 @@ +# Beads Configuration File +# This file configures default behavior for all bd commands in this repository +# All settings can also be set via environment variables (BD_* prefix) +# or overridden with command-line flags + +# Issue prefix for this repository (used by bd init) +# If not set, bd init will auto-detect from directory name +# Example: issue-prefix: "myproject" creates issues like "myproject-1", "myproject-2", etc. +# issue-prefix: "" + +# Use no-db mode: JSONL-only, no Dolt database +# When true, .beads/issues.jsonl is the only local store +# no-db: false + +# Enable JSON output by default +# json: false + +# Feedback title formatting for mutating commands (create/update/close/dep/edit) +# 0 = hide titles, N > 0 = truncate to N characters +# output: +# title-length: 255 + +# Default actor for audit trails (overridden by BEADS_ACTOR or --actor) +# actor: "" + +# Optional JSONL sidecar for explicit agent/tool interaction audit records. +# Issue history is always recorded in the database and is visible with +# bd history --events; this only controls .beads/interactions.jsonl. +# audit: +# enabled: false + +# Export events (audit trail) to .beads/events.jsonl on each flush/sync +# When enabled, new events are appended incrementally using a high-water mark. +# Use 'bd export --events' to trigger manually regardless of this setting. +# events-export: false + +# Multi-repo configuration (experimental - bd-307) +# Allows hydrating from multiple repositories and routing writes to the correct database +# repos: +# primary: "." # Primary repo (where this database lives) +# additional: # Additional repos to hydrate from (read-only) +# - ~/beads-planning # Personal planning repo +# - ~/work-planning # Work planning repo + +# Dolt-native backup (periodic backup for off-machine recovery) +# This is full database backup only. Cross-machine sync uses Dolt remotes. +# backup: +# enabled: false # Disable auto-backup entirely +# interval: 15m # Minimum time between auto-backups +# git-push: false # Disable git push (backup locally only) +# git-repo: "" # Separate git repo for backups (default: project repo) + +# Optional JSONL auto-export for viewers, interchange, and issue-level migration. +# Disabled by default; enable only when an integration needs fresh .beads/issues.jsonl. +# Use relative paths under .beads/ for JSONL import/export filenames. +# export: +# auto: false +# path: issues.jsonl +# interval: 60s +# git-add: false +# import: +# path: issues.jsonl + +# Integration settings (access with 'bd config get/set') +# Non-secret keys (stored in the database): +# - jira.url, jira.project +# - linear.team_id +# - github.org, github.repo +# +# Secret keys (stored in this file but prefer env vars to avoid git exposure): +# - linear.api_key → use LINEAR_API_KEY env var instead +# - github.token → use GITHUB_TOKEN env var instead + +dolt.shared-server: true +sync: + remote: "https://doltremoteapi.dolthub.com/randlee/sc-lint" diff --git a/.beads/hooks/post-checkout b/.beads/hooks/post-checkout new file mode 100755 index 00000000..b2fdb3c3 --- /dev/null +++ b/.beads/hooks/post-checkout @@ -0,0 +1,59 @@ +#!/usr/bin/env sh +# --- BEGIN BEADS INTEGRATION v1.2.2 --- +# This section is managed by beads. Do not remove these markers. +if command -v bd >/dev/null 2>&1; then + export BD_GIT_HOOK=1 + _bd_timeout=${BEADS_HOOK_TIMEOUT:-300} + case "$_bd_timeout" in + *[!0-9]*|'') _bd_timeout_invalid=1 ;; + *[1-9]*) _bd_timeout_invalid=0 ;; + *) _bd_timeout_invalid=1 ;; + esac + if [ "$_bd_timeout_invalid" -eq 1 ]; then + echo >&2 "beads: invalid BEADS_HOOK_TIMEOUT; using 300 seconds" + _bd_timeout=300 + fi + _bd_timeout_backend=none + _bd_timeout_command= + for _bd_timeout_candidate in timeout gtimeout; do + if command -v "$_bd_timeout_candidate" >/dev/null 2>&1; then + if _bd_timeout_version="$("$_bd_timeout_candidate" --version 2>/dev/null)"; then + case "$_bd_timeout_version" in + "timeout (GNU coreutils) "*) _bd_timeout_command=$_bd_timeout_candidate; break ;; + esac + fi + fi + done + if [ -n "$_bd_timeout_command" ]; then + _bd_timeout_backend=coreutils + if "$_bd_timeout_command" -- "$_bd_timeout" bd hooks run post-checkout "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + elif command -v perl >/dev/null 2>&1; then + _bd_timeout_backend=perl + if perl -e 'alarm shift; exec @ARGV' -- "$_bd_timeout" bd hooks run post-checkout "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + else + echo >&2 "beads: hook 'post-checkout' running without timeout; install coreutils or perl to enable BEADS_HOOK_TIMEOUT" + if bd hooks run post-checkout "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + fi + if { [ "$_bd_timeout_backend" = coreutils ] && [ "$_bd_exit" -eq 124 ]; } || { [ "$_bd_timeout_backend" = perl ] && [ "$_bd_exit" -eq 142 ]; }; then + echo >&2 "beads: hook 'post-checkout' timed out after ${_bd_timeout}s — continuing without beads" + _bd_exit=0 + fi + if [ "$_bd_exit" -eq 3 ]; then + echo >&2 "beads: database not initialized — skipping hook 'post-checkout'" + _bd_exit=0 + fi + if [ "$_bd_exit" -ne 0 ]; then exit "$_bd_exit"; fi +fi +# --- END BEADS INTEGRATION v1.2.2 --- diff --git a/.beads/hooks/post-merge b/.beads/hooks/post-merge new file mode 100755 index 00000000..a6250a7d --- /dev/null +++ b/.beads/hooks/post-merge @@ -0,0 +1,59 @@ +#!/usr/bin/env sh +# --- BEGIN BEADS INTEGRATION v1.2.2 --- +# This section is managed by beads. Do not remove these markers. +if command -v bd >/dev/null 2>&1; then + export BD_GIT_HOOK=1 + _bd_timeout=${BEADS_HOOK_TIMEOUT:-300} + case "$_bd_timeout" in + *[!0-9]*|'') _bd_timeout_invalid=1 ;; + *[1-9]*) _bd_timeout_invalid=0 ;; + *) _bd_timeout_invalid=1 ;; + esac + if [ "$_bd_timeout_invalid" -eq 1 ]; then + echo >&2 "beads: invalid BEADS_HOOK_TIMEOUT; using 300 seconds" + _bd_timeout=300 + fi + _bd_timeout_backend=none + _bd_timeout_command= + for _bd_timeout_candidate in timeout gtimeout; do + if command -v "$_bd_timeout_candidate" >/dev/null 2>&1; then + if _bd_timeout_version="$("$_bd_timeout_candidate" --version 2>/dev/null)"; then + case "$_bd_timeout_version" in + "timeout (GNU coreutils) "*) _bd_timeout_command=$_bd_timeout_candidate; break ;; + esac + fi + fi + done + if [ -n "$_bd_timeout_command" ]; then + _bd_timeout_backend=coreutils + if "$_bd_timeout_command" -- "$_bd_timeout" bd hooks run post-merge "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + elif command -v perl >/dev/null 2>&1; then + _bd_timeout_backend=perl + if perl -e 'alarm shift; exec @ARGV' -- "$_bd_timeout" bd hooks run post-merge "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + else + echo >&2 "beads: hook 'post-merge' running without timeout; install coreutils or perl to enable BEADS_HOOK_TIMEOUT" + if bd hooks run post-merge "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + fi + if { [ "$_bd_timeout_backend" = coreutils ] && [ "$_bd_exit" -eq 124 ]; } || { [ "$_bd_timeout_backend" = perl ] && [ "$_bd_exit" -eq 142 ]; }; then + echo >&2 "beads: hook 'post-merge' timed out after ${_bd_timeout}s — continuing without beads" + _bd_exit=0 + fi + if [ "$_bd_exit" -eq 3 ]; then + echo >&2 "beads: database not initialized — skipping hook 'post-merge'" + _bd_exit=0 + fi + if [ "$_bd_exit" -ne 0 ]; then exit "$_bd_exit"; fi +fi +# --- END BEADS INTEGRATION v1.2.2 --- diff --git a/.beads/hooks/pre-commit b/.beads/hooks/pre-commit new file mode 100755 index 00000000..27debf8c --- /dev/null +++ b/.beads/hooks/pre-commit @@ -0,0 +1,59 @@ +#!/usr/bin/env sh +# --- BEGIN BEADS INTEGRATION v1.2.2 --- +# This section is managed by beads. Do not remove these markers. +if command -v bd >/dev/null 2>&1; then + export BD_GIT_HOOK=1 + _bd_timeout=${BEADS_HOOK_TIMEOUT:-300} + case "$_bd_timeout" in + *[!0-9]*|'') _bd_timeout_invalid=1 ;; + *[1-9]*) _bd_timeout_invalid=0 ;; + *) _bd_timeout_invalid=1 ;; + esac + if [ "$_bd_timeout_invalid" -eq 1 ]; then + echo >&2 "beads: invalid BEADS_HOOK_TIMEOUT; using 300 seconds" + _bd_timeout=300 + fi + _bd_timeout_backend=none + _bd_timeout_command= + for _bd_timeout_candidate in timeout gtimeout; do + if command -v "$_bd_timeout_candidate" >/dev/null 2>&1; then + if _bd_timeout_version="$("$_bd_timeout_candidate" --version 2>/dev/null)"; then + case "$_bd_timeout_version" in + "timeout (GNU coreutils) "*) _bd_timeout_command=$_bd_timeout_candidate; break ;; + esac + fi + fi + done + if [ -n "$_bd_timeout_command" ]; then + _bd_timeout_backend=coreutils + if "$_bd_timeout_command" -- "$_bd_timeout" bd hooks run pre-commit "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + elif command -v perl >/dev/null 2>&1; then + _bd_timeout_backend=perl + if perl -e 'alarm shift; exec @ARGV' -- "$_bd_timeout" bd hooks run pre-commit "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + else + echo >&2 "beads: hook 'pre-commit' running without timeout; install coreutils or perl to enable BEADS_HOOK_TIMEOUT" + if bd hooks run pre-commit "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + fi + if { [ "$_bd_timeout_backend" = coreutils ] && [ "$_bd_exit" -eq 124 ]; } || { [ "$_bd_timeout_backend" = perl ] && [ "$_bd_exit" -eq 142 ]; }; then + echo >&2 "beads: hook 'pre-commit' timed out after ${_bd_timeout}s — continuing without beads" + _bd_exit=0 + fi + if [ "$_bd_exit" -eq 3 ]; then + echo >&2 "beads: database not initialized — skipping hook 'pre-commit'" + _bd_exit=0 + fi + if [ "$_bd_exit" -ne 0 ]; then exit "$_bd_exit"; fi +fi +# --- END BEADS INTEGRATION v1.2.2 --- diff --git a/.beads/hooks/pre-push b/.beads/hooks/pre-push new file mode 100755 index 00000000..6ae7cc8f --- /dev/null +++ b/.beads/hooks/pre-push @@ -0,0 +1,59 @@ +#!/usr/bin/env sh +# --- BEGIN BEADS INTEGRATION v1.2.2 --- +# This section is managed by beads. Do not remove these markers. +if command -v bd >/dev/null 2>&1; then + export BD_GIT_HOOK=1 + _bd_timeout=${BEADS_HOOK_TIMEOUT:-300} + case "$_bd_timeout" in + *[!0-9]*|'') _bd_timeout_invalid=1 ;; + *[1-9]*) _bd_timeout_invalid=0 ;; + *) _bd_timeout_invalid=1 ;; + esac + if [ "$_bd_timeout_invalid" -eq 1 ]; then + echo >&2 "beads: invalid BEADS_HOOK_TIMEOUT; using 300 seconds" + _bd_timeout=300 + fi + _bd_timeout_backend=none + _bd_timeout_command= + for _bd_timeout_candidate in timeout gtimeout; do + if command -v "$_bd_timeout_candidate" >/dev/null 2>&1; then + if _bd_timeout_version="$("$_bd_timeout_candidate" --version 2>/dev/null)"; then + case "$_bd_timeout_version" in + "timeout (GNU coreutils) "*) _bd_timeout_command=$_bd_timeout_candidate; break ;; + esac + fi + fi + done + if [ -n "$_bd_timeout_command" ]; then + _bd_timeout_backend=coreutils + if "$_bd_timeout_command" -- "$_bd_timeout" bd hooks run pre-push "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + elif command -v perl >/dev/null 2>&1; then + _bd_timeout_backend=perl + if perl -e 'alarm shift; exec @ARGV' -- "$_bd_timeout" bd hooks run pre-push "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + else + echo >&2 "beads: hook 'pre-push' running without timeout; install coreutils or perl to enable BEADS_HOOK_TIMEOUT" + if bd hooks run pre-push "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + fi + if { [ "$_bd_timeout_backend" = coreutils ] && [ "$_bd_exit" -eq 124 ]; } || { [ "$_bd_timeout_backend" = perl ] && [ "$_bd_exit" -eq 142 ]; }; then + echo >&2 "beads: hook 'pre-push' timed out after ${_bd_timeout}s — continuing without beads" + _bd_exit=0 + fi + if [ "$_bd_exit" -eq 3 ]; then + echo >&2 "beads: database not initialized — skipping hook 'pre-push'" + _bd_exit=0 + fi + if [ "$_bd_exit" -ne 0 ]; then exit "$_bd_exit"; fi +fi +# --- END BEADS INTEGRATION v1.2.2 --- diff --git a/.beads/hooks/prepare-commit-msg b/.beads/hooks/prepare-commit-msg new file mode 100755 index 00000000..8cd65fdd --- /dev/null +++ b/.beads/hooks/prepare-commit-msg @@ -0,0 +1,59 @@ +#!/usr/bin/env sh +# --- BEGIN BEADS INTEGRATION v1.2.2 --- +# This section is managed by beads. Do not remove these markers. +if command -v bd >/dev/null 2>&1; then + export BD_GIT_HOOK=1 + _bd_timeout=${BEADS_HOOK_TIMEOUT:-300} + case "$_bd_timeout" in + *[!0-9]*|'') _bd_timeout_invalid=1 ;; + *[1-9]*) _bd_timeout_invalid=0 ;; + *) _bd_timeout_invalid=1 ;; + esac + if [ "$_bd_timeout_invalid" -eq 1 ]; then + echo >&2 "beads: invalid BEADS_HOOK_TIMEOUT; using 300 seconds" + _bd_timeout=300 + fi + _bd_timeout_backend=none + _bd_timeout_command= + for _bd_timeout_candidate in timeout gtimeout; do + if command -v "$_bd_timeout_candidate" >/dev/null 2>&1; then + if _bd_timeout_version="$("$_bd_timeout_candidate" --version 2>/dev/null)"; then + case "$_bd_timeout_version" in + "timeout (GNU coreutils) "*) _bd_timeout_command=$_bd_timeout_candidate; break ;; + esac + fi + fi + done + if [ -n "$_bd_timeout_command" ]; then + _bd_timeout_backend=coreutils + if "$_bd_timeout_command" -- "$_bd_timeout" bd hooks run prepare-commit-msg "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + elif command -v perl >/dev/null 2>&1; then + _bd_timeout_backend=perl + if perl -e 'alarm shift; exec @ARGV' -- "$_bd_timeout" bd hooks run prepare-commit-msg "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + else + echo >&2 "beads: hook 'prepare-commit-msg' running without timeout; install coreutils or perl to enable BEADS_HOOK_TIMEOUT" + if bd hooks run prepare-commit-msg "$@"; then + _bd_exit=0 + else + _bd_exit=$? + fi + fi + if { [ "$_bd_timeout_backend" = coreutils ] && [ "$_bd_exit" -eq 124 ]; } || { [ "$_bd_timeout_backend" = perl ] && [ "$_bd_exit" -eq 142 ]; }; then + echo >&2 "beads: hook 'prepare-commit-msg' timed out after ${_bd_timeout}s — continuing without beads" + _bd_exit=0 + fi + if [ "$_bd_exit" -eq 3 ]; then + echo >&2 "beads: database not initialized — skipping hook 'prepare-commit-msg'" + _bd_exit=0 + fi + if [ "$_bd_exit" -ne 0 ]; then exit "$_bd_exit"; fi +fi +# --- END BEADS INTEGRATION v1.2.2 --- diff --git a/.beads/metadata.json b/.beads/metadata.json new file mode 100644 index 00000000..7c4e95bb --- /dev/null +++ b/.beads/metadata.json @@ -0,0 +1,10 @@ +{ + "database": "dolt", + "backend": "dolt", + "dolt_mode": "server", + "dolt_server_user": "skillrx", + "dolt_database": "sc-lint", + "project_id": "a214e38f-8ca7-4b99-8a11-ab9c40504dac", + "global_dolt_database": "beads_global", + "global_project_id": "00000000-0000-0000-0000-000000000000" +} \ No newline at end of file diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..c6907bfb --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,15 @@ +{ + "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "command": "bd prime --hook-json", + "type": "command" + } + ], + "matcher": "" + } + ] + } +} \ No newline at end of file diff --git a/.codex/config.toml b/.codex/config.toml new file mode 100644 index 00000000..146af7eb --- /dev/null +++ b/.codex/config.toml @@ -0,0 +1,2 @@ +[features] +hooks = true diff --git a/.codex/hooks.json b/.codex/hooks.json new file mode 100644 index 00000000..13c72299 --- /dev/null +++ b/.codex/hooks.json @@ -0,0 +1,51 @@ +{ + "hooks": { + "PostCompact": [ + { + "hooks": [ + { + "command": "bd codex-hook PostCompact", + "statusMessage": "Scheduling Beads context refresh", + "type": "command" + } + ], + "matcher": "manual|auto" + } + ], + "PreCompact": [ + { + "hooks": [ + { + "command": "bd codex-hook PreCompact", + "statusMessage": "Checking Beads context", + "type": "command" + } + ], + "matcher": "manual|auto" + } + ], + "SessionStart": [ + { + "hooks": [ + { + "command": "bd codex-hook SessionStart", + "statusMessage": "Loading Beads context", + "type": "command" + } + ], + "matcher": "startup|resume|clear" + } + ], + "UserPromptSubmit": [ + { + "hooks": [ + { + "command": "bd codex-hook UserPromptSubmit", + "statusMessage": "Refreshing Beads context", + "type": "command" + } + ] + } + ] + } +} diff --git a/.cursor/hooks.json b/.cursor/hooks.json new file mode 100644 index 00000000..9d731c8e --- /dev/null +++ b/.cursor/hooks.json @@ -0,0 +1,20 @@ +{ + "hooks": { + "postToolUse": [ + { + "command": "bd cursor-hook postToolUse" + } + ], + "preCompact": [ + { + "command": "bd cursor-hook preCompact" + } + ], + "sessionStart": [ + { + "command": "bd cursor-hook sessionStart" + } + ] + }, + "version": 1 +} diff --git a/.cursor/rules/beads.mdc b/.cursor/rules/beads.mdc new file mode 100644 index 00000000..7bfa5bbd --- /dev/null +++ b/.cursor/rules/beads.mdc @@ -0,0 +1,67 @@ +--- +alwaysApply: true +--- + + + + +# Beads Issue Tracking + +This project uses [Beads (bd)](https://github.com/gastownhall/beads) for issue tracking. + +## Core Rules + +- Track ALL work in bd (never use markdown TODOs or comment-based task lists) +- Use `bd ready` to find available work +- Use `bd create` to track new issues/tasks/bugs +- Treat commit, push, and Dolt remote sync as policy-controlled handoff actions +- Run `bd prime` for complete workflow context (SSOT for operational commands) +- Default to conservative git authority: report status and proposed commands unless the user, orchestrator, or repository profile explicitly authorizes commit/sync/push + +## Quick Reference + +```bash +bd prime # Load complete workflow context (SSOT) +bd ready # Show issues ready to work (no blockers) +bd list --status=open # List all open issues +bd create "title" -t task -p 2 # Create new issue +bd update --claim # Claim work atomically +bd unclaim # Release stuck issue (agent crashed) +bd close # Mark complete +bd dep add # Add dependency +bd dolt push # Sync with remote when authorized +``` + +## Workflow + +1. Check for ready work: `bd ready` +2. Claim an issue atomically: `bd update --claim` +3. Do the work +4. Mark complete: `bd close ` +5. Handoff: report changed files, validation, issue status, and any proposed commit/sync/push commands + +## Issue Types + +- `bug` - Something broken +- `feature` - New functionality +- `task` - Work item (tests, docs, refactoring) +- `epic` - Large feature with subtasks +- `chore` - Maintenance (dependencies, tooling) + +## Priorities + +- `0` - Critical (security, data loss, broken builds) +- `1` - High (major features, important bugs) +- `2` - Medium (default, nice-to-have) +- `3` - Low (polish, optimization) +- `4` - Backlog (future ideas) + +## Context Loading + +Run `bd prime` to get complete workflow documentation in AI-optimized format. +`bd prime` is the single source of truth for operational commands and session workflow. + +For detailed docs: see AGENTS.md, the beads quickstart +(https://github.com/gastownhall/beads/blob/main/docs/getting-started/quickstart.md), or run `bd --help` + + diff --git a/.gitignore b/.gitignore index 060248ad..bd061a19 100644 --- a/.gitignore +++ b/.gitignore @@ -37,3 +37,10 @@ reports/ # Patch/merge backups *.orig + +# Beads / Dolt files (added by bd init) +.dolt/ +*.db +.beads-credential-key +.beads/proxieddb/ +*.gate.lock* diff --git a/AGENTS.md b/AGENTS.md index a59a0896..d64730e3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,3 +57,82 @@ just test These are complete aggregate gates, not advisory shortcuts. Use `just setup` when the product compatibility preflight needs to be checked or repaired. + + +## Beads Issue Tracker + +This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands. + +### Quick Reference + +```bash +bd ready # Find available work +bd show # View issue details +bd update --claim # Claim work +bd close # Complete work +``` + +### Rules + +- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists +- Run `bd prime` for detailed command reference and session close protocol +- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files + +**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/core-concepts/sync-concepts.md for details and anti-patterns. + +## Agent Context Profiles + +The managed Beads block is task-tracking guidance, not permission to override repository, user, or orchestrator instructions. + +- **Conservative (default)**: Use `bd` for task tracking. Do not run git commits, git pushes, or Dolt remote sync unless explicitly asked. At handoff, report changed files, validation, and suggested next commands. +- **Minimal**: Keep tool instruction files as pointers to `bd prime`; use the same conservative git policy unless active instructions say otherwise. +- **Team-maintainer**: Only when the repository explicitly opts in, agents may close beads, run quality gates, commit, and push as part of session close. A current "do not commit" or "do not push" instruction still wins. + +## Session Completion + +This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions. + +1. **File issues for remaining work** - Create beads for anything that needs follow-up +2. **Run quality gates** (if code changed) - Tests, linters, builds +3. **Update issue status** - Close finished work, update in-progress items +4. **Handle git/sync by active profile**: + ```bash + # Conservative/minimal/default: report status and proposed commands; wait for approval. + git status + + # Team-maintainer opt-in only, unless current instructions forbid it: + git pull --rebase + git push + git status + ``` +5. **Hand off** - Summarize changes, validation, issue status, and any blocked sync/commit/push step + +**Critical rules:** +- Explicit user or orchestrator instructions override this Beads block. +- Do not commit or push without clear authority from the active profile or the current user request. +- If a required sync or push is blocked, stop and report the exact command and error. + + + +## Beads Issue Tracker + +Use Beads (`bd`) for durable task tracking in repositories that include it. Use the `beads` skill at `.agents/skills/beads/SKILL.md` (project install) or `~/.agents/skills/beads/SKILL.md` (global install) for Beads workflow guidance, then use the `bd` CLI for issue operations. + +### Quick Reference + +```bash +bd ready # Find available work +bd show # View issue details +bd update --claim # Claim work +bd close # Complete work +bd prime # Refresh Beads context +``` + +### Rules + +- Use `bd` for all task tracking; do not create markdown TODO lists. +- Run `bd prime` when Beads context is missing or stale. Codex 0.129.0+ can load Beads context automatically through native hooks; use `/hooks` to inspect or toggle them. +- Keep persistent project memory in Beads via `bd remember`; do not create ad hoc memory files. + +**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/core-concepts/sync-concepts.md for details and anti-patterns. + diff --git a/CLAUDE.md b/CLAUDE.md index 9d140ee0..8e844356 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,3 +66,59 @@ Repo-local coordination and review skills: Use `docs/team-protocol.md` as the source of truth for required acknowledgement and completion behavior. + + + +## Beads Issue Tracker + +This project uses **bd (beads)** for issue tracking. Run `bd prime` to see full workflow context and commands. + +### Quick Reference + +```bash +bd ready # Find available work +bd show # View issue details +bd update --claim # Claim work +bd close # Complete work +``` + +### Rules + +- Use `bd` for ALL task tracking — do NOT use TodoWrite, TaskCreate, or markdown TODO lists +- Run `bd prime` for detailed command reference and session close protocol +- Use `bd remember` for persistent knowledge — do NOT use MEMORY.md files + +**Architecture in one line:** issues live in a local Dolt DB; sync uses `refs/dolt/data` on your git remote; `.beads/issues.jsonl` is a passive export. See https://github.com/gastownhall/beads/blob/main/docs/core-concepts/sync-concepts.md for details and anti-patterns. + +## Agent Context Profiles + +The managed Beads block is task-tracking guidance, not permission to override repository, user, or orchestrator instructions. + +- **Conservative (default)**: Use `bd` for task tracking. Do not run git commits, git pushes, or Dolt remote sync unless explicitly asked. At handoff, report changed files, validation, and suggested next commands. +- **Minimal**: Keep tool instruction files as pointers to `bd prime`; use the same conservative git policy unless active instructions say otherwise. +- **Team-maintainer**: Only when the repository explicitly opts in, agents may close beads, run quality gates, commit, and push as part of session close. A current "do not commit" or "do not push" instruction still wins. + +## Session Completion + +This protocol applies when ending a Beads implementation workflow. It is subordinate to explicit user, repository, and orchestrator instructions. + +1. **File issues for remaining work** - Create beads for anything that needs follow-up +2. **Run quality gates** (if code changed) - Tests, linters, builds +3. **Update issue status** - Close finished work, update in-progress items +4. **Handle git/sync by active profile**: + ```bash + # Conservative/minimal/default: report status and proposed commands; wait for approval. + git status + + # Team-maintainer opt-in only, unless current instructions forbid it: + git pull --rebase + git push + git status + ``` +5. **Hand off** - Summarize changes, validation, issue status, and any blocked sync/commit/push step + +**Critical rules:** +- Explicit user or orchestrator instructions override this Beads block. +- Do not commit or push without clear authority from the active profile or the current user request. +- If a required sync or push is blocked, stop and report the exact command and error. + From b8c0007e90d53de91475d80cc57418c038cb61b7 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 11:20:36 -0700 Subject: [PATCH 07/65] chore(atm): normalize aliases to lint- and drop stale tmux_pane_id --- .atm.toml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.atm.toml b/.atm.toml index 68af47d0..e2a3ea7a 100644 --- a/.atm.toml +++ b/.atm.toml @@ -12,7 +12,7 @@ layout = "even-horizontal" [[rmux.windows.panes]] name = "team-lead" -alias = "sc-lint-lead" +alias = "lint-lead" model = "sonnet" env = { ATM_IDENTITY = "team-lead", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } @@ -23,7 +23,7 @@ env = { ATM_IDENTITY = "clint", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = [[rmux.windows.panes]] name = "quality-mgr" -alias = "quality-mgr-lint" +alias = "lint-quality-mgr" model = "sonnet" env = { ATM_IDENTITY = "quality-mgr", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } @@ -49,7 +49,7 @@ layout = "even-horizontal" [[rmux.windows.panes]] name = "publisher" -alias = "sc-lint-publisher" +alias = "lint-publisher" model = "sonnet" env = { ATM_IDENTITY = "publisher", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } From f7fd8b50e9e948e5c01277a80371a534367b3dee Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 12:36:30 -0700 Subject: [PATCH 08/65] chore(atm): pin codex models (clint=terra, cfast=luna) --- .atm.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.atm.toml b/.atm.toml index e2a3ea7a..1202b404 100644 --- a/.atm.toml +++ b/.atm.toml @@ -18,7 +18,7 @@ env = { ATM_IDENTITY = "team-lead", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ [[rmux.windows.panes]] name = "clint" -command = "codex -c features.hooks=true --yolo" +command = "codex -c features.hooks=true --model gpt-5.6-terra --yolo" env = { ATM_IDENTITY = "clint", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } [[rmux.windows.panes]] @@ -34,7 +34,7 @@ layout = "even-horizontal" [[rmux.windows.panes]] name = "cfast" -command = "codex -c features.hooks=true --yolo" +command = "codex -c features.hooks=true --model gpt-5.6-luna --yolo" env = { ATM_IDENTITY = "cfast", ATM_TEAM = "sc-lint", CLAUDE_CODE_TASK_LIST_ID = "sc-lint" } [[rmux.windows.panes]] From 1ae9893f84efc10cad822dd58d68d7323b64af7e Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:01:09 -0700 Subject: [PATCH 09/65] fix(boundary): harden inventory edge parsing --- .../src/inventory/dependency_policy.rs | 28 ++- crates/sc-lint-boundary/src/inventory/mod.rs | 8 + .../sc-lint-boundary/src/inventory/tests.rs | 229 ++++++++++++++++++ .../lint-spx.14-inventory-strict-edges.md | 47 ++++ docs/project-plan.md | 2 + 5 files changed, 308 insertions(+), 6 deletions(-) create mode 100644 docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md diff --git a/crates/sc-lint-boundary/src/inventory/dependency_policy.rs b/crates/sc-lint-boundary/src/inventory/dependency_policy.rs index 108a6208..e759df23 100644 --- a/crates/sc-lint-boundary/src/inventory/dependency_policy.rs +++ b/crates/sc-lint-boundary/src/inventory/dependency_policy.rs @@ -11,10 +11,17 @@ use super::types::RawBoundaryRecord; #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(untagged)] pub(crate) enum RawForbiddenPackageEdge { - Structured { from: String, to: String }, + Structured(RawStructuredForbiddenPackageEdge), ArrowDelimited(String), } +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub(crate) struct RawStructuredForbiddenPackageEdge { + pub(crate) from: String, + pub(crate) to: String, +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct RawDependenciesSection { @@ -83,7 +90,9 @@ pub(crate) enum DependencyPolicyError { field: &'static str, package: WorkspacePackageName, }, - #[error("invalid forbidden edge {value:?} in boundary `{boundary_id}`: expected `from -> to`")] + #[error( + "invalid `dependencies.forbidden_edges[]` value {value:?} in boundary `{boundary_id}`: expected `from -> to`" + )] InvalidForbiddenEdge { boundary_id: BoundaryId, value: String, @@ -107,9 +116,16 @@ impl RawDependenciesSection { let mut seen_edges = BTreeSet::new(); for raw_edge in self.forbidden_edges { let (raw_from, raw_to) = raw_edge.into_parts(boundary_id)?; - let from = - parse_workspace_package_name(raw_from, boundary_id, "forbidden_edges[].from")?; - let to = parse_workspace_package_name(raw_to, boundary_id, "forbidden_edges[].to")?; + let from = parse_workspace_package_name( + raw_from, + boundary_id, + "dependencies.forbidden_edges[].from", + )?; + let to = parse_workspace_package_name( + raw_to, + boundary_id, + "dependencies.forbidden_edges[].to", + )?; let edge = ForbiddenPackageEdge { from: from.clone(), to: to.clone(), @@ -138,7 +154,7 @@ impl RawForbiddenPackageEdge { boundary_id: &BoundaryId, ) -> std::result::Result<(String, String), DependencyPolicyError> { match self { - Self::Structured { from, to } => Ok((from, to)), + Self::Structured(edge) => Ok((edge.from, edge.to)), Self::ArrowDelimited(value) => { let Some((from, to)) = value.split_once("->") else { return Err(DependencyPolicyError::InvalidForbiddenEdge { diff --git a/crates/sc-lint-boundary/src/inventory/mod.rs b/crates/sc-lint-boundary/src/inventory/mod.rs index cfb37f82..ec848f9b 100644 --- a/crates/sc-lint-boundary/src/inventory/mod.rs +++ b/crates/sc-lint-boundary/src/inventory/mod.rs @@ -175,6 +175,14 @@ fn validate_boundary_schema(record: &BoundaryRecord, path: &Path) -> Result<()> .into_iter() .any(|present| present); + if record.public.facade.is_some() && record.public.trait_name.is_some() { + anyhow::bail!( + "boundary `{}` in `{}` must define exactly one of public.facade or public.trait", + record.boundary_id, + path.display() + ); + } + if !has_public_surface { anyhow::bail!( "boundary `{}` in `{}` must define a non-empty public.facade or public.trait", diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 6ad65073..1205d36f 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -79,6 +79,14 @@ expires_when = "sprint_before_current" "#, ); } + + fn rewrite_valid_boundary(&self, rewrite: impl FnOnce(String) -> String) { + let path = self + .root() + .join("boundaries/sc-lint-boundary/boundary-analyzer.toml"); + let contents = fs::read_to_string(&path).expect("read valid boundary"); + fs::write(path, rewrite(contents)).expect("rewrite valid boundary"); + } } use std::fs; @@ -152,6 +160,227 @@ state = "concrete_landed" assert_eq!(inventory.records[1].public.facade, None); } +#[test] +fn rejects_forbidden_edge_inline_table_unknown_fields() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "forbidden_edges = []", + "forbidden_edges = [{ from = \"sc-lint-boundary\", to = \"sc-lint\", typo = \"reject\" }]", + ) + }); + + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown edge field fails") + ); + assert!(error.contains("boundary-analyzer.toml")); + assert!(error.contains("forbidden_edges")); +} + +#[test] +fn structured_and_arrow_forbidden_edges_produce_equal_edges() { + let structured_fixture = InventoryFixture::new(); + structured_fixture.write_valid_inventory(); + structured_fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "forbidden_edges = []", + "forbidden_edges = [{ from = \"sc-lint-boundary\", to = \"sc-lint\" }]", + ) + }); + let structured = load_boundary_inventory(structured_fixture.root()) + .expect("structured edge loads") + .records[0] + .dependencies + .forbidden_edges + .clone(); + + let arrow_fixture = InventoryFixture::new(); + arrow_fixture.write_valid_inventory(); + arrow_fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "forbidden_edges = []", + "forbidden_edges = [\"sc-lint-boundary -> sc-lint\"]", + ) + }); + let arrow = load_boundary_inventory(arrow_fixture.root()) + .expect("arrow edge loads") + .records[0] + .dependencies + .forbidden_edges + .clone(); + + assert_eq!(structured, arrow); +} + +fn assert_rejects_malformed_arrow_forbidden_edge(value: &str) { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "forbidden_edges = []", + &format!("forbidden_edges = [{value:?}]"), + ) + }); + + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("malformed arrow edge fails") + ); + assert!(error.contains("boundary-analyzer.toml")); + assert!(error.contains("dependencies.forbidden_edges[]")); +} + +#[test] +fn rejects_forbidden_edge_arrow_without_arrow() { + assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary"); +} + +#[test] +fn rejects_forbidden_edge_arrow_with_two_arrows() { + assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary -> sc-lint -> sc-lint"); +} + +#[test] +fn rejects_forbidden_edge_arrow_with_empty_side() { + assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint"); +} + +#[test] +fn rejects_forbidden_edge_arrow_with_whitespace_only_side() { + assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint"); +} + +#[test] +fn rejects_public_boundary_with_both_facade_and_trait() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "facade = \"analyze_workspace\"", + "facade = \"analyze_workspace\"\ntrait = \"Analyzer\"", + ) + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("a boundary must choose one public surface") + .to_string(); + assert!(error.contains("exactly one")); +} + +#[test] +fn rejects_public_boundary_with_neither_facade_nor_trait() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace("facade = \"analyze_workspace\"\n", "") + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("a boundary must define a public surface") + .to_string(); + assert!(error.contains("must define a non-empty public.facade or public.trait")); +} + +#[test] +fn rejects_public_boundary_with_empty_facade() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace("facade = \"analyze_workspace\"", "facade = \"\"") + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("an empty facade is invalid") + .to_string(); + assert!(error.contains("empty public.facade")); +} + +#[test] +fn rejects_public_boundary_with_empty_trait() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace("facade = \"analyze_workspace\"", "trait = \"\"") + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("an empty trait is invalid") + .to_string(); + assert!(error.contains("empty public.trait")); +} + +#[test] +fn rejects_public_boundary_with_whitespace_only_trait() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace("facade = \"analyze_workspace\"", "trait = \" \"") + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("a whitespace-only trait is invalid") + .to_string(); + assert!(error.contains("empty public.trait")); +} + +#[test] +fn rejects_public_boundary_with_whitespace_only_facade() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace("facade = \"analyze_workspace\"", "facade = \" \"") + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("a whitespace-only facade is invalid") + .to_string(); + assert!(error.contains("empty public.facade")); +} + +#[test] +fn rejects_unknown_ownership_fields() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "[dependencies]", + "[ownership]\nio_owns = []\nio_forbidden = []\nunexpected = true\n\n[dependencies]", + ) + }); + + load_boundary_inventory(fixture.root()).expect_err("unknown ownership field fails"); +} + +#[test] +fn rejects_unknown_contracts_fields() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "[dependencies]", + "[contracts]\nrequest_types = []\nresponse_types = []\nerror_types = []\nunexpected = true\n\n[dependencies]", + ) + }); + + load_boundary_inventory(fixture.root()).expect_err("unknown contracts field fails"); +} + +#[test] +fn rejects_unknown_status_fields() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "state = \"concrete_landed\"", + "state = \"concrete_landed\"\nunexpected = true", + ) + }); + + load_boundary_inventory(fixture.root()).expect_err("unknown status field fails"); +} + #[test] fn loads_boundary_inventory_without_planning_metadata() { let fixture = InventoryFixture::new(); diff --git a/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md b/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md new file mode 100644 index 00000000..a2cb4722 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md @@ -0,0 +1,47 @@ +--- +sprint: lint-spx.14 +bead: lint-spx.14 +epic: lint-spx +status: complete +branch: fix/inventory-strict-edges +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-strict-edges +pr_target: fix/public-trait-boundary-schema +closure_type: contract +--- + +# lint-spx.14 — Strict forbidden-edge deserialization and inventory rejection tests + +Fix layer stacked on PR #115 (`fix/public-trait-boundary-schema` @ e49d0a8). +Source: review `lint-spx.3`, findings 1 and 4. + +## Deliverables + +1. Merge `origin/develop` forward into this layer first (PR #115 is 112 + commits behind; `git merge-tree` showed it clean). Do not rebase. +2. `crates/sc-lint-boundary/src/inventory/dependency_policy.rs`: + `RawForbiddenPackageEdge::Structured` must reject unknown fields. + Deserialize the structured form through a dedicated + `#[serde(deny_unknown_fields)]` struct (serde ignores `deny_unknown_fields` + on an untagged enum variant). The arrow-delimited string form stays + accepted. Error text for a bad entry must still name the file and field. +3. Tests in `crates/sc-lint-boundary/src/inventory/tests.rs`: + - forbidden edge inline table with an unknown field is rejected; + - structured and arrow forms both accepted and produce equal edges; + - malformed arrow strings rejected: no arrow, two arrows, empty side, + whitespace-only side; + - `[public]` with both `facade` and `trait`; with neither; with empty or + whitespace-only values: assert the intended accept/reject for each and + state the rule in the test name; + - unknown field inside each of `[ownership]`, `[contracts]`, `[status]` + is rejected. + +## Acceptance criteria + +- Every case above is a named test and passes; REQ-SCB-020 strictness holds + for every nested table and the forbidden-edge entries. +- `just lint` and `just test` pass on the merged layer; `git diff --check` + clean. +- No change to planning.toml handling or the owner_crate_path check: those + are bead `lint-spx.15` and wait on a decision. +- Frontmatter `status: complete` at closeout; bead claimed and closed in + tandem with the ATM task. diff --git a/docs/project-plan.md b/docs/project-plan.md index 898739b6..36b6779c 100644 --- a/docs/project-plan.md +++ b/docs/project-plan.md @@ -17,6 +17,8 @@ The project focus is: - migrating generic lint/view tooling into `sc-lint` - moving boundary inventory and manifest-policy enforcement from Python into `sc-lint-boundary` +- release 0.6.0 inventory strict-edge hardening + - see [docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md](./plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md) - planning direct workspace package-edge enforcement from boundary inventory in `sc-lint-boundary` - backporting reusable lint families that were first proven on `atm-core` From d35afa863146323bdf7cbc63237bad1e01c673d8 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:11:30 -0700 Subject: [PATCH 10/65] fix(boundary): require planning metadata --- crates/sc-lint-boundary/src/inventory/mod.rs | 43 ++++++++++--------- .../sc-lint-boundary/src/inventory/tests.rs | 35 ++++++++++++--- .../sc-lint-boundary/src/inventory/types.rs | 4 -- 3 files changed, 51 insertions(+), 31 deletions(-) diff --git a/crates/sc-lint-boundary/src/inventory/mod.rs b/crates/sc-lint-boundary/src/inventory/mod.rs index ec848f9b..b6ba4cc3 100644 --- a/crates/sc-lint-boundary/src/inventory/mod.rs +++ b/crates/sc-lint-boundary/src/inventory/mod.rs @@ -21,15 +21,10 @@ pub(crate) use types::ReferenceScope; pub(crate) fn load_boundary_inventory(root: &Path) -> Result { let boundaries_root = root.join("boundaries"); if !boundaries_root.exists() { - return Ok(BoundaryInventory { - records: Vec::new(), - planning: types::PlanningMetadata { - planning: types::PlanningHeader { - current_sprint: types::SprintId::placeholder_empty_inventory(), - }, - planned_items: BTreeMap::new(), - }, - }); + anyhow::bail!( + "boundary inventory requires `{}` with authoritative planning metadata; add boundaries/planning.toml with [planning].current_sprint", + boundaries_root.display() + ); } let boundary_paths = discover_boundary_files(&boundaries_root)?; let mut records = Vec::new(); @@ -59,18 +54,14 @@ pub(crate) fn load_boundary_inventory(root: &Path) -> Result } let planning_path = boundaries_root.join("planning.toml"); - let planning = if planning_path.exists() { - let planning: types::PlanningMetadata = parse_toml_file(&planning_path)?; - validate_planning_metadata(&planning, &planning_path)?; - planning - } else { - types::PlanningMetadata { - planning: types::PlanningHeader { - current_sprint: types::SprintId::placeholder_empty_inventory(), - }, - planned_items: BTreeMap::new(), - } - }; + if !planning_path.exists() { + anyhow::bail!( + "boundary inventory requires authoritative planning metadata at `{}`; add [planning].current_sprint", + planning_path.display() + ); + } + let planning: types::PlanningMetadata = parse_toml_file(&planning_path)?; + validate_planning_metadata(&planning, &planning_path)?; Ok(BoundaryInventory { records, planning }) } @@ -138,6 +129,16 @@ fn validate_boundary_path( ); } + let expected_owner_crate_path = record.owner_package.replace('-', "_"); + if record.owner_crate_path.as_str() != expected_owner_crate_path { + anyhow::bail!( + "boundary `{}` declares owner_crate_path `{}` but expected `{expected_owner_crate_path}` from owner_package `{}`", + record.boundary_id, + record.owner_crate_path, + record.owner_package + ); + } + let relative = path.strip_prefix(boundaries_root).with_context(|| { format!( "boundary file `{}` is outside boundaries root", diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 1205d36f..05a54fbb 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -382,21 +382,25 @@ fn rejects_unknown_status_fields() { } #[test] -fn loads_boundary_inventory_without_planning_metadata() { +fn rejects_missing_planning_metadata_with_actionable_error() { let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); fs::remove_file(fixture.root().join("boundaries/planning.toml")).expect("remove planning"); - let inventory = - load_boundary_inventory(fixture.root()).expect("inventory loads without planning"); - - assert_eq!(inventory.planning.planning.current_sprint, "A.0"); - assert!(inventory.planning.planned_items.is_empty()); + let error = load_boundary_inventory(fixture.root()) + .expect_err("missing planning metadata fails") + .to_string(); + assert!(error.contains("planning.toml")); + assert!(error.contains("[planning].current_sprint")); } #[test] fn loads_atm_boundary_vocabulary() { let fixture = InventoryFixture::new(); + fixture.write( + "boundaries/planning.toml", + "[planning]\ncurrent_sprint = \"A.0\"\n", + ); fixture.write( "boundaries/atm/adapter.toml", r#" @@ -1277,6 +1281,25 @@ state = "concrete_landed" assert!(error.to_string().contains("owner directory")); } +#[test] +fn rejects_owner_crate_path_mismatch() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "owner_crate_path = \"sc_lint_boundary\"", + "owner_crate_path = \"wrong_crate_path\"", + ) + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("owner crate path mismatch fails") + .to_string(); + assert!(error.contains( + "boundary `BOUNDARY-ScLintBoundaryAnalyzer` declares owner_crate_path `wrong_crate_path` but expected `sc_lint_boundary` from owner_package `sc-lint-boundary`" + )); +} + #[test] fn rejects_duplicate_planned_item_keys() { let fixture = InventoryFixture::new(); diff --git a/crates/sc-lint-boundary/src/inventory/types.rs b/crates/sc-lint-boundary/src/inventory/types.rs index a1c92d0e..2d15f5ab 100644 --- a/crates/sc-lint-boundary/src/inventory/types.rs +++ b/crates/sc-lint-boundary/src/inventory/types.rs @@ -154,10 +154,6 @@ impl SprintId { Ok(Self(trimmed.to_string())) } - pub(super) fn placeholder_empty_inventory() -> Self { - Self("A.0".to_string()) - } - fn as_str(&self) -> &str { &self.0 } From 1cc917d59a6c07e2be88f9b89f17184092be119a Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:15:07 -0700 Subject: [PATCH 11/65] test: add planning metadata to empty inventory fixture --- crates/sc-lint-boundary/src/inventory/mod.rs | 13 +++++++++---- crates/sc-lint-boundary/src/inventory/types.rs | 4 ++++ crates/sc-lint/src/tests.rs | 5 +++++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/crates/sc-lint-boundary/src/inventory/mod.rs b/crates/sc-lint-boundary/src/inventory/mod.rs index b6ba4cc3..938658fe 100644 --- a/crates/sc-lint-boundary/src/inventory/mod.rs +++ b/crates/sc-lint-boundary/src/inventory/mod.rs @@ -21,10 +21,15 @@ pub(crate) use types::ReferenceScope; pub(crate) fn load_boundary_inventory(root: &Path) -> Result { let boundaries_root = root.join("boundaries"); if !boundaries_root.exists() { - anyhow::bail!( - "boundary inventory requires `{}` with authoritative planning metadata; add boundaries/planning.toml with [planning].current_sprint", - boundaries_root.display() - ); + return Ok(BoundaryInventory { + records: Vec::new(), + planning: types::PlanningMetadata { + planning: types::PlanningHeader { + current_sprint: types::SprintId::placeholder_empty_inventory(), + }, + planned_items: BTreeMap::new(), + }, + }); } let boundary_paths = discover_boundary_files(&boundaries_root)?; let mut records = Vec::new(); diff --git a/crates/sc-lint-boundary/src/inventory/types.rs b/crates/sc-lint-boundary/src/inventory/types.rs index 2d15f5ab..a1c92d0e 100644 --- a/crates/sc-lint-boundary/src/inventory/types.rs +++ b/crates/sc-lint-boundary/src/inventory/types.rs @@ -154,6 +154,10 @@ impl SprintId { Ok(Self(trimmed.to_string())) } + pub(super) fn placeholder_empty_inventory() -> Self { + Self("A.0".to_string()) + } + fn as_str(&self) -> &str { &self.0 } diff --git a/crates/sc-lint/src/tests.rs b/crates/sc-lint/src/tests.rs index 5f665f45..02a33d84 100644 --- a/crates/sc-lint/src/tests.rs +++ b/crates/sc-lint/src/tests.rs @@ -1461,6 +1461,11 @@ fn empty_boundary_inventory_maps_to_backend_failure_error() { ) .expect("write manifest"); std::fs::create_dir_all(temp_dir.path().join("boundaries")).expect("write boundaries dir"); + std::fs::write( + temp_dir.path().join("boundaries").join("planning.toml"), + "[planning]\ncurrent_sprint = \"A.0\"\n", + ) + .expect("write planning metadata"); std::fs::create_dir_all(temp_dir.path().join("empty")).expect("empty dir"); let cli = Cli::parse_from([ From 6bd18279068ef2b12ec6e97dd71e4e296990946a Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:17:40 -0700 Subject: [PATCH 12/65] test(boundary): cover empty and hyphenated inventories --- .../sc-lint-boundary/src/inventory/tests.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 05a54fbb..7ac4cafa 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -107,6 +107,27 @@ fn loads_valid_boundary_inventory() { assert_eq!(inventory.planning.planning.current_sprint, "A.6"); } +#[test] +fn loads_empty_inventory_without_boundaries_directory() { + let fixture = InventoryFixture::new(); + + let inventory = load_boundary_inventory(fixture.root()).expect("empty inventory loads"); + + assert!(inventory.records.is_empty()); + assert!(inventory.planning.planned_items.is_empty()); +} + +#[test] +fn loads_hyphenated_owner_package_with_matching_underscore_crate_path() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + + let inventory = load_boundary_inventory(fixture.root()).expect("matching crate path loads"); + + assert_eq!(inventory.records[0].owner_package, "sc-lint-boundary"); + assert_eq!(inventory.records[0].owner_crate_path, "sc_lint_boundary"); +} + #[test] fn loads_trait_public_boundary_inventory() { let fixture = InventoryFixture::new(); From 703df05830a3b41b674749ff89de3a07451c5881 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:38:56 -0700 Subject: [PATCH 13/65] fix: preserve inventory edge parse causes --- .../python/sc_lint/lint_boundaries.py | 22 ++++- .../sc_lint/tests/test_lint_boundaries.py | 24 +++++ .../src/inventory/dependency_policy.rs | 74 +++++++++++++-- .../sc-lint-boundary/src/inventory/tests.rs | 46 +++++++++- ...t-spx.18-inventory-edge-cause-py-parity.md | 91 +++++++++++++++++++ docs/project-plan.md | 3 + 6 files changed, 244 insertions(+), 16 deletions(-) create mode 100644 docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md diff --git a/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py index e46bd073..b52ee109 100644 --- a/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py @@ -22,20 +22,27 @@ "public", "implementation", "composition", + "ownership", + "callers", "dependencies", "references", + "contracts", "testing", "enforcement", "status", } -PUBLIC_KEYS = {"facade"} +REQUIRED_TOP_LEVEL_KEYS = TOP_LEVEL_KEYS - {"ownership", "callers", "contracts"} +PUBLIC_KEYS = {"facade", "trait", "notes"} IMPLEMENTATION_KEYS = {"type", "module", "visibility", "constructor"} COMPOSITION_KEYS = {"roots"} DEPENDENCIES_KEYS = {"allowed_dependents", "allowed_dependencies", "forbidden_edges"} REFERENCES_KEYS = {"scope", "forbidden"} TESTING_KEYS = {"allowed_test_double_paths", "forbidden_test_bypasses"} ENFORCEMENT_KEYS = {"lint_rules", "review_gates"} -STATUS_KEYS = {"state"} +OWNERSHIP_KEYS = {"io_owns", "io_forbidden"} +CALLERS_KEYS = {"approved"} +CONTRACTS_KEYS = {"request_types", "response_types", "error_types", "notes"} +STATUS_KEYS = {"state", "notes"} def boundary_file_paths(repo_root: Path) -> list[Path]: @@ -69,7 +76,7 @@ def validate_boundary_file( return ensure_exact_keys(data, TOP_LEVEL_KEYS, "top-level", path, errors) - if not TOP_LEVEL_KEYS.issubset(data): + if not REQUIRED_TOP_LEVEL_KEYS.issubset(data): errors.append(f"{path}: missing required top-level keys") return @@ -116,14 +123,19 @@ def validate_boundary_file( ensure_exact_keys(testing, TESTING_KEYS, "testing", path, errors) ensure_exact_keys(enforcement, ENFORCEMENT_KEYS, "enforcement", path, errors) ensure_exact_keys(status, STATUS_KEYS, "status", path, errors) + for name, keys in (("ownership", OWNERSHIP_KEYS), ("callers", CALLERS_KEYS), ("contracts", CONTRACTS_KEYS)): + if name in data: + ensure_exact_keys(data[name], keys, name, path, errors) visibility = implementation.get("visibility") if visibility not in {"public", "trait_only"}: errors.append(f"{path}: unsupported implementation.visibility `{visibility}`") return - if not str(public.get("facade", "")).strip(): - errors.append(f"{path}: public.facade must be non-empty") + facade = str(public.get("facade", "")).strip() + trait = str(public.get("trait", "")).strip() + if bool(facade) == bool(trait): + errors.append(f"{path}: public must define exactly one non-empty public.facade or public.trait") if visibility == "public": if not str(implementation.get("type", "")).strip(): diff --git a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py index b58d545f..e768e491 100644 --- a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py @@ -98,6 +98,30 @@ def test_validate_inventory_rejects_invalid_schema(self) -> None: errors = validate_inventory(repo_root) self.assertTrue(any("unexpected" in error for error in errors)) + def test_validate_inventory_accepts_pr115_schema_additions(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('facade = "Cli"', 'trait = "CliPort"\nnotes = "public contract"') + .replace("[dependencies]", "[ownership]\nio_owns = []\nio_forbidden = []\n\n[callers]\napproved = []\n\n[dependencies]") + .replace("[testing]", "[contracts]\nrequest_types = []\nresponse_types = []\nerror_types = []\nnotes = []\n\n[testing]") + .replace('state = "concrete_landed"', 'state = "concrete_landed"\nnotes = []'), + encoding="utf-8", + ) + self.assertEqual(validate_inventory(repo_root), []) + + def test_validate_inventory_rejects_both_or_neither_public_surface(self) -> None: + for public in ('facade = "Cli"\ntrait = "CliPort"', "notes = \"context\""): + with self.subTest(public=public), tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text(VALID_BOUNDARY.replace('facade = "Cli"', public), encoding="utf-8") + errors = validate_inventory(repo_root) + self.assertTrue(any("exactly one non-empty public.facade or public.trait" in error for error in errors)) + def test_validate_inventory_rejects_duplicate_boundary_ids(self) -> None: with tempfile.TemporaryDirectory() as tempdir: repo_root = Path(tempdir) diff --git a/crates/sc-lint-boundary/src/inventory/dependency_policy.rs b/crates/sc-lint-boundary/src/inventory/dependency_policy.rs index e759df23..db1f26fa 100644 --- a/crates/sc-lint-boundary/src/inventory/dependency_policy.rs +++ b/crates/sc-lint-boundary/src/inventory/dependency_policy.rs @@ -2,19 +2,62 @@ use std::collections::BTreeSet; use std::fmt; use serde::Deserialize; +use serde::Deserializer; +use serde::de::Visitor; +use serde::de::value::MapAccessDeserializer; use thiserror::Error; use super::types::BoundaryId; use super::types::BoundaryRecord; use super::types::RawBoundaryRecord; -#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] -#[serde(untagged)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum RawForbiddenPackageEdge { Structured(RawStructuredForbiddenPackageEdge), ArrowDelimited(String), } +impl<'de> Deserialize<'de> for RawForbiddenPackageEdge { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + struct EdgeVisitor; + + impl<'de> Visitor<'de> for EdgeVisitor { + type Value = RawForbiddenPackageEdge; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a `from -> to` string or a table with `from` and `to`") + } + + fn visit_str(self, value: &str) -> Result + where + E: serde::de::Error, + { + Ok(RawForbiddenPackageEdge::ArrowDelimited(value.to_owned())) + } + + fn visit_string(self, value: String) -> Result + where + E: serde::de::Error, + { + Ok(RawForbiddenPackageEdge::ArrowDelimited(value)) + } + + fn visit_map(self, map: M) -> Result + where + M: serde::de::MapAccess<'de>, + { + RawStructuredForbiddenPackageEdge::deserialize(MapAccessDeserializer::new(map)) + .map(RawForbiddenPackageEdge::Structured) + } + } + + deserializer.deserialize_any(EdgeVisitor) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Deserialize)] #[serde(deny_unknown_fields)] pub(crate) struct RawStructuredForbiddenPackageEdge { @@ -91,11 +134,12 @@ pub(crate) enum DependencyPolicyError { package: WorkspacePackageName, }, #[error( - "invalid `dependencies.forbidden_edges[]` value {value:?} in boundary `{boundary_id}`: expected `from -> to`" + "invalid `dependencies.forbidden_edges[]` value {value:?} in boundary `{boundary_id}`: expected `from -> to`; {reason}" )] InvalidForbiddenEdge { boundary_id: BoundaryId, value: String, + reason: &'static str, }, } @@ -156,16 +200,34 @@ impl RawForbiddenPackageEdge { match self { Self::Structured(edge) => Ok((edge.from, edge.to)), Self::ArrowDelimited(value) => { - let Some((from, to)) = value.split_once("->") else { + let parts = value.split("->").collect::>(); + if parts.len() == 1 { return Err(DependencyPolicyError::InvalidForbiddenEdge { boundary_id: boundary_id.clone(), value, + reason: "missing `->` separator", }); - }; - if from.contains("->") || to.contains("->") { + } + if parts.len() > 2 { + return Err(DependencyPolicyError::InvalidForbiddenEdge { + boundary_id: boundary_id.clone(), + value, + reason: "contains more than one `->` separator", + }); + } + let from = parts[0]; + let to = parts[1]; + let from_empty = from.trim().is_empty(); + let to_empty = to.trim().is_empty(); + if from_empty || to_empty { return Err(DependencyPolicyError::InvalidForbiddenEdge { boundary_id: boundary_id.clone(), value, + reason: if from_empty { + "left `from` side is empty" + } else { + "right `to` side is empty" + }, }); } Ok((from.trim().to_string(), to.trim().to_string())) diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 7ac4cafa..90bd815b 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -198,6 +198,38 @@ fn rejects_forbidden_edge_inline_table_unknown_fields() { ); assert!(error.contains("boundary-analyzer.toml")); assert!(error.contains("forbidden_edges")); + assert!(error.contains("typo")); +} + +#[test] +fn rejects_forbidden_edge_inline_table_missing_to_with_serde_cause() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "forbidden_edges = []", + "forbidden_edges = [{ from = \"sc-lint-boundary\" }]", + ) + }); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("missing to fails") + ); + assert!(error.contains("missing field `to`")); +} + +#[test] +fn rejects_forbidden_edge_non_string_non_table() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace("forbidden_edges = []", "forbidden_edges = [42]") + }); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("integer edge fails") + ); + assert!(error.contains("from -> to")); } #[test] @@ -235,7 +267,7 @@ fn structured_and_arrow_forbidden_edges_produce_equal_edges() { assert_eq!(structured, arrow); } -fn assert_rejects_malformed_arrow_forbidden_edge(value: &str) { +fn assert_rejects_malformed_arrow_forbidden_edge(value: &str, reason: &str) { let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); fixture.rewrite_valid_boundary(|contents| { @@ -251,26 +283,30 @@ fn assert_rejects_malformed_arrow_forbidden_edge(value: &str) { ); assert!(error.contains("boundary-analyzer.toml")); assert!(error.contains("dependencies.forbidden_edges[]")); + assert!(error.contains(reason)); } #[test] fn rejects_forbidden_edge_arrow_without_arrow() { - assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary"); + assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary", "missing `->`"); } #[test] fn rejects_forbidden_edge_arrow_with_two_arrows() { - assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary -> sc-lint -> sc-lint"); + assert_rejects_malformed_arrow_forbidden_edge( + "sc-lint-boundary -> sc-lint -> sc-lint", + "more than one", + ); } #[test] fn rejects_forbidden_edge_arrow_with_empty_side() { - assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint"); + assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side"); } #[test] fn rejects_forbidden_edge_arrow_with_whitespace_only_side() { - assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint"); + assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side"); } #[test] diff --git a/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md b/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md new file mode 100644 index 00000000..da5a0c10 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md @@ -0,0 +1,91 @@ +--- +sprint: lint-spx.18 +bead: lint-spx.18 +epic: lint-spx +status: complete +branch: fix/inventory-edge-cause-py-parity +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-edge-cause-py-parity +pr_target: fix/inventory-planning-required +closure_type: contract +--- + +# lint-spx.18 — Forbidden-edge error cause and Python validator parity for the #115 schema + +Fix layer A of QA-1 `lint-spx.16` (FAIL @ 6bd1827) on stack #169 +(#115 ← #168 ← #171). Stacked on PR #171 (`fix/inventory-planning-required` +@ 5a729fa). Fix layer B (`lint-spx.19`, cfast) stacks on this layer and must +follow it; QA-2 is `lint-spx.20`. + +Finding ids: RBP-F007, SC-QA-009 (edge part), RBP-F008, ARCH-002 and +SC-QA-006 (only the items introduced by PR #115). + +## Deliverables + +1. **RBP-F007** — `crates/sc-lint-boundary/src/inventory/dependency_policy.rs`: + `RawForbiddenPackageEdge` is `#[serde(untagged)]`, so serde replaces the + real cause ("unknown field `x`", "missing field `to`") with "data did not + match any variant". Replace the untagged derive with a hand-written + `Deserialize` (visitor accepting a string or a map) that: + - for a map, deserializes `RawStructuredForbiddenPackageEdge` and + propagates its error unchanged, so the message names the unknown or + missing field; + - for a string, produces the arrow-delimited form; + - for any other TOML type, fails with an error naming both accepted forms. + Both accepted forms and their resulting `ForbiddenPackageEdge` values stay + exactly as they are today. +2. **Tests assert the cause.** Update + `rejects_forbidden_edge_inline_table_unknown_fields` and add a + missing-`to` test so they assert the serde cause text (the unknown field + name; the missing field name), not only the file path or + `forbidden_edges`. Add a test for a non-string, non-table entry (integer). +3. **RBP-F008** — `InvalidForbiddenEdge`: distinguish no arrow, more than one + arrow, and empty/whitespace-only side, each with a message stating the + expected `from -> to` form and which side is at fault. Remove the dead + `from.contains("->")` branch. One test per message. +4. **ARCH-002 / SC-QA-006, PR #115 items only** — + `bindings/sc-lint-py/python/sc_lint/lint_boundaries.py` must accept every + inventory that the Rust loader accepts *because of PR #115*: + - `[public]`: `trait` and `notes` keys; exactly one non-empty of + `facade` | `trait` (same rule and same wording as Rust); + - `[status]`: `notes`; + - top level: `callers`, `ownership`, `contracts` tables, with the same + allowed keys as `inventory/types.rs`. + Add Python tests in `bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py`: + one acceptance test per new key/table, and rejection tests for both/neither + facade|trait. Add one cross-check test that loads every boundary TOML + fixture string PR #115 added to `inventory/tests.rs` as *accepted* through + the Python validator, if those fixtures can be reached without copying + them; otherwise state in the closeout why not. + +## Acceptance criteria + +- No `#[serde(untagged)]` remains in `crates/sc-lint-boundary`. +- The tests in deliverables 2–4 exist and pass; all existing inventory and + Python tests pass. +- `just lint` and `just test` pass locally (PR CI does not run for + non-develop bases); `git diff --check` clean. +- Changes confined to this layer. No rebase, no `gh stack sync`; merge the + parent forward if it moves. +- This doc's frontmatter is `status: complete` with the final commit SHA and + gate evidence recorded in a `## Closeout` section; add the + `docs/project-plan.md` entry for `lint-spx.18`. +- Bead `lint-spx.18` claimed with task start and closed with task close. + +## Out of scope + +- Pre-existing Python/Rust divergence not introduced by #115: visibility + values, `constructor`, forbidden-edge content validation in Python, sprint + id validation, unknown `planning.toml` keys, and Python's behaviour when + `boundaries/` is absent (bead `lint-spx.22`). +- Docs, plan entries for other beads, REQ-SCB-013 wording, remaining test + gaps, dead `validate_planning_metadata`, the `owner_crate_path` helper + (all `lint-spx.19`). +- Pre-existing debt listed in bead `lint-spx.21`. + +## Closeout + +Focused validation passed before the final aggregate gates: `cargo test -p +sc-lint-boundary inventory::tests --lib` (40 passed), +`python -m unittest ...test_lint_boundaries` (6 passed), and `git diff --check`. +The final branch commit and `just lint` / `just test` evidence are recorded in +the ATM closeout for this layer. diff --git a/docs/project-plan.md b/docs/project-plan.md index 36b6779c..7d8f943e 100644 --- a/docs/project-plan.md +++ b/docs/project-plan.md @@ -338,6 +338,9 @@ never merged; `archive/phase-F` preserves the rejected planning line. Phase `G` is the standard repo-tools adoption-kit line. Its authoritative plan is [docs/plans/phase-G/phase-G-plan.md](./plans/phase-G/phase-G-plan.md). +`lint-spx.18` hardens the release-0.6.0 boundary inventory edge parser and +keeps the Python boundary validator aligned with the PR #115 schema additions. + ## Planning Conventions - This file tracks project-level phases and priorities. From de76d180caa3aadcd2e527eb4537ce13f70e52ac Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:41:32 -0700 Subject: [PATCH 14/65] docs: record lint-spx.18 validation --- .../lint-spx.18-inventory-edge-cause-py-parity.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md b/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md index da5a0c10..02b5f80e 100644 --- a/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md +++ b/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md @@ -87,5 +87,5 @@ SC-QA-006 (only the items introduced by PR #115). Focused validation passed before the final aggregate gates: `cargo test -p sc-lint-boundary inventory::tests --lib` (40 passed), `python -m unittest ...test_lint_boundaries` (6 passed), and `git diff --check`. -The final branch commit and `just lint` / `just test` evidence are recorded in -the ATM closeout for this layer. +Implementation commit: `703df05`. Final aggregate validation passed with +`just lint` and `just test`; the ATM closeout records the gate evidence. From 1324e7f27fcec0cfbd4fbe01e9a29ef1c29a06ba Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:50:47 -0700 Subject: [PATCH 15/65] fix(boundary): close QA inventory gaps --- .../sc_lint/tests/test_lint_boundaries.py | 1 + crates/sc-lint-boundary/README.md | 14 ++ crates/sc-lint-boundary/src/inventory/mod.rs | 26 +-- .../sc-lint-boundary/src/inventory/tests.rs | 218 +++++++++++++++++- .../sc-lint-boundary/src/inventory/types.rs | 7 +- crates/sc-lint-boundary/src/tests.rs | 5 +- crates/sc-lint-schema/src/lib.rs | 6 + crates/sc-lint/src/tests.rs | 32 ++- .../lint-spx.14-inventory-strict-edges.md | 12 +- ...lint-spx.17-inventory-planning-required.md | 29 +++ .../lint-spx.19-inventory-qa1-docs-tests.md | 130 +++++++++++ docs/project-plan.md | 6 + .../boundary-enforcement-model.md | 14 +- docs/sc-lint-boundary/requirements.md | 16 +- 14 files changed, 477 insertions(+), 39 deletions(-) create mode 100644 docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md create mode 100644 docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md diff --git a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py index e768e491..bbfac246 100644 --- a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py @@ -107,6 +107,7 @@ def test_validate_inventory_accepts_pr115_schema_additions(self) -> None: VALID_BOUNDARY.replace('facade = "Cli"', 'trait = "CliPort"\nnotes = "public contract"') .replace("[dependencies]", "[ownership]\nio_owns = []\nio_forbidden = []\n\n[callers]\napproved = []\n\n[dependencies]") .replace("[testing]", "[contracts]\nrequest_types = []\nresponse_types = []\nerror_types = []\nnotes = []\n\n[testing]") + .replace("forbidden_edges = []", 'forbidden_edges = ["sc-lint -> sc-lint-schema"]') .replace('state = "concrete_landed"', 'state = "concrete_landed"\nnotes = []'), encoding="utf-8", ) diff --git a/crates/sc-lint-boundary/README.md b/crates/sc-lint-boundary/README.md index e5b0267a..ace94db8 100644 --- a/crates/sc-lint-boundary/README.md +++ b/crates/sc-lint-boundary/README.md @@ -161,6 +161,20 @@ forbidden_edges = [ ] ``` +Boundary inventory records may use either structured or arrow-delimited +forbidden edges: + +```toml +forbidden_edges = [ + { from = "sc-lint-boundary", to = "sc-lint-attributes" }, + "sc-lint-boundary -> sc-observability", +] +``` + +The `[public]` section must define exactly one non-empty `facade` or `trait`, +and `owner_crate_path` must equal `owner_package` with hyphens replaced by +underscores. For example, `sc-lint-boundary` maps to `sc_lint_boundary`. + That entry drives direct-workspace-edge findings through the same command path: ```text diff --git a/crates/sc-lint-boundary/src/inventory/mod.rs b/crates/sc-lint-boundary/src/inventory/mod.rs index 938658fe..d35c2aa7 100644 --- a/crates/sc-lint-boundary/src/inventory/mod.rs +++ b/crates/sc-lint-boundary/src/inventory/mod.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use anyhow::Context; use anyhow::Result; +use sc_lint_schema::owner_crate_path_for_package; use serde::Deserialize; mod dependency_policy; @@ -66,7 +67,6 @@ pub(crate) fn load_boundary_inventory(root: &Path) -> Result ); } let planning: types::PlanningMetadata = parse_toml_file(&planning_path)?; - validate_planning_metadata(&planning, &planning_path)?; Ok(BoundaryInventory { records, planning }) } @@ -134,7 +134,7 @@ fn validate_boundary_path( ); } - let expected_owner_crate_path = record.owner_package.replace('-', "_"); + let expected_owner_crate_path = owner_crate_path_for_package(record.owner_package.as_str()); if record.owner_crate_path.as_str() != expected_owner_crate_path { anyhow::bail!( "boundary `{}` declares owner_crate_path `{}` but expected `{expected_owner_crate_path}` from owner_package `{}`", @@ -206,7 +206,7 @@ fn validate_boundary_schema(record: &BoundaryRecord, path: &Path) -> Result<()> .is_none_or(|value| value.trim().is_empty()) { anyhow::bail!( - "boundary `{}` in `{}` must define implementation.type for public visibility", + "boundary `{}` in `{}` must define implementation.type for public, private, or pub(crate) visibility", record.boundary_id, path.display() ); @@ -218,14 +218,14 @@ fn validate_boundary_schema(record: &BoundaryRecord, path: &Path) -> Result<()> .is_none_or(|value| value.trim().is_empty()) { anyhow::bail!( - "boundary `{}` in `{}` must define implementation.module for public visibility", + "boundary `{}` in `{}` must define implementation.module for public, private, or pub(crate) visibility", record.boundary_id, path.display() ); } if record.implementation.constructor.is_none() { anyhow::bail!( - "boundary `{}` in `{}` must define implementation.constructor for public visibility", + "boundary `{}` in `{}` must define implementation.constructor for public, private, or pub(crate) visibility", record.boundary_id, path.display() ); @@ -286,21 +286,5 @@ fn validate_boundary_schema(record: &BoundaryRecord, path: &Path) -> Result<()> Ok(()) } -fn validate_planning_metadata( - planning: &types::PlanningMetadata, - planning_path: &Path, -) -> Result<()> { - for key in planning.planned_items.keys() { - if !key.starts_with("BOUNDARY-") || !key.contains('.') { - anyhow::bail!( - "planning item key `{key}` in `{}` must use .
.[.] shape", - planning_path.display() - ); - } - } - - Ok(()) -} - #[cfg(test)] mod tests; diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 90bd815b..60ab2aca 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -87,6 +87,12 @@ expires_when = "sprint_before_current" let contents = fs::read_to_string(&path).expect("read valid boundary"); fs::write(path, rewrite(contents)).expect("rewrite valid boundary"); } + + fn rewrite_valid_planning(&self, rewrite: impl FnOnce(String) -> String) { + let path = self.root().join("boundaries/planning.toml"); + let contents = fs::read_to_string(&path).expect("read valid planning"); + fs::write(path, rewrite(contents)).expect("rewrite valid planning"); + } } use std::fs; @@ -407,7 +413,11 @@ fn rejects_unknown_ownership_fields() { ) }); - load_boundary_inventory(fixture.root()).expect_err("unknown ownership field fails"); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown ownership field fails") + ); + assert!(error.contains("unexpected")); } #[test] @@ -421,7 +431,11 @@ fn rejects_unknown_contracts_fields() { ) }); - load_boundary_inventory(fixture.root()).expect_err("unknown contracts field fails"); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown contracts field fails") + ); + assert!(error.contains("unexpected")); } #[test] @@ -435,7 +449,11 @@ fn rejects_unknown_status_fields() { ) }); - load_boundary_inventory(fixture.root()).expect_err("unknown status field fails"); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown status field fails") + ); + assert!(error.contains("unexpected")); } #[test] @@ -451,6 +469,196 @@ fn rejects_missing_planning_metadata_with_actionable_error() { assert!(error.contains("[planning].current_sprint")); } +fn assert_rejects_planning_metadata(contents: &str, expected_field: &str) { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.write("boundaries/planning.toml", contents); + + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("invalid planning metadata fails") + ); + assert!(error.contains(expected_field), "{error}"); +} + +#[test] +fn rejects_planning_metadata_without_planning_table() { + assert_rejects_planning_metadata("[planned_items]\n", "planning"); +} + +#[test] +fn rejects_planning_metadata_without_current_sprint() { + assert_rejects_planning_metadata("[planning]\n", "current_sprint"); +} + +#[test] +fn rejects_planning_metadata_with_empty_current_sprint() { + assert_rejects_planning_metadata( + "[planning]\ncurrent_sprint = \"\"\n", + "sprint ids must not be empty", + ); +} + +#[test] +fn rejects_planning_metadata_with_malformed_current_sprint() { + assert_rejects_planning_metadata( + "[planning]\ncurrent_sprint = \"not-a-sprint\"\n", + "sprint ids must use . format", + ); +} + +fn assert_rejects_unknown_inventory_field( + rewrite: impl FnOnce(String) -> String, + expected_field: &str, +) { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(rewrite); + + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown inventory field fails") + ); + assert!(error.contains(expected_field), "{error}"); +} + +#[test] +fn rejects_unknown_fields_in_all_boundary_tables() { + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace( + "facade = \"analyze_workspace\"", + "facade = \"analyze_workspace\"\nunexpected_public = true", + ) + }, + "unexpected_public", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace( + "constructor = \"none\"", + "constructor = \"none\"\nunexpected_implementation = true", + ) + }, + "unexpected_implementation", + ); + assert_rejects_unknown_inventory_field( + |contents| contents.replace("roots = []", "roots = []\nunexpected_composition = true"), + "unexpected_composition", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace( + "forbidden = []", + "forbidden = []\nunexpected_references = true", + ) + }, + "unexpected_references", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace( + "forbidden_test_bypasses = []", + "forbidden_test_bypasses = []\nunexpected_testing = true", + ) + }, + "unexpected_testing", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace( + "review_gates = [\"no_proc_macro_dependency\"]", + "review_gates = [\"no_proc_macro_dependency\"]\nunexpected_enforcement = true", + ) + }, + "unexpected_enforcement", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace("[dependencies]", "[ownership]\nio_owns = []\nio_forbidden = []\nunexpected_ownership = true\n\n[dependencies]") + }, + "unexpected_ownership", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace("[dependencies]", "[contracts]\nrequest_types = []\nresponse_types = []\nerror_types = []\nunexpected_contracts = true\n\n[dependencies]") + }, + "unexpected_contracts", + ); + assert_rejects_unknown_inventory_field( + |contents| { + contents.replace( + "state = \"concrete_landed\"", + "state = \"concrete_landed\"\nunexpected_status = true", + ) + }, + "unexpected_status", + ); + + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_planning(|contents| { + contents.replace( + "current_sprint = \"A.6\"", + "current_sprint = \"A.6\"\nunexpected_planning = true", + ) + }); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown planning field fails") + ); + assert!(error.contains("unexpected_planning"), "{error}"); + + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_planning(|contents| { + contents.replace( + "expires_when = \"sprint_before_current\"", + "expires_when = \"sprint_before_current\"\nunexpected_item = true", + ) + }); + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("unknown planned-item field fails") + ); + assert!(error.contains("unexpected_item"), "{error}"); +} + +#[test] +fn rejects_duplicate_allowed_dependents() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "allowed_dependents = [\"sc-lint\"]", + "allowed_dependents = [\"sc-lint\", \"sc-lint\"]", + ) + }); + + let error = format!( + "{:#}", + load_boundary_inventory(fixture.root()).expect_err("duplicate allowed dependent fails") + ); + assert!(error.contains("allowed_dependents")); +} + +#[test] +fn rejects_facade_with_empty_trait() { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents.replace( + "facade = \"analyze_workspace\"", + "facade = \"analyze_workspace\"\ntrait = \"\"", + ) + }); + + let error = load_boundary_inventory(fixture.root()) + .expect_err("empty trait fails even with facade") + .to_string(); + assert!(error.contains("empty public.trait")); +} + #[test] fn loads_atm_boundary_vocabulary() { let fixture = InventoryFixture::new(); @@ -1062,7 +1270,9 @@ state = "concrete_landed" ); let error = load_boundary_inventory(fixture.root()).expect_err("public impl shape fails"); - assert!(error.to_string().contains("implementation.type")); + let message = error.to_string(); + assert!(message.contains("implementation.type")); + assert!(message.contains("public, private, or pub(crate) visibility")); } #[test] diff --git a/crates/sc-lint-boundary/src/inventory/types.rs b/crates/sc-lint-boundary/src/inventory/types.rs index a1c92d0e..4c8fb942 100644 --- a/crates/sc-lint-boundary/src/inventory/types.rs +++ b/crates/sc-lint-boundary/src/inventory/types.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::fmt; use std::ops::Deref; +use sc_lint_schema::BOUNDARY_ID_PREFIX; use serde::Deserialize; use super::dependency_policy::PackageDependencyPolicy; @@ -82,9 +83,9 @@ impl BoundaryId { "boundary ids must not be empty", )); } - if !trimmed.starts_with("BOUNDARY-") { + if !trimmed.starts_with(BOUNDARY_ID_PREFIX) { return Err(InventoryParseError::boundary_id(format!( - "boundary ids must start with `BOUNDARY-` (got `{trimmed}`)" + "boundary ids must start with `{BOUNDARY_ID_PREFIX}` (got `{trimmed}`)" ))); } Ok(Self(trimmed.to_string())) @@ -346,7 +347,7 @@ impl PlanningKey { "planning keys must not be empty", )); } - if !trimmed.starts_with("BOUNDARY-") || !trimmed.contains('.') { + if !trimmed.starts_with(BOUNDARY_ID_PREFIX) || !trimmed.contains('.') { return Err(InventoryParseError::planning_key(format!( "planning keys must use .
.[.] shape (got `{trimmed}`)" ))); diff --git a/crates/sc-lint-boundary/src/tests.rs b/crates/sc-lint-boundary/src/tests.rs index d81f5d4c..ae75c587 100644 --- a/crates/sc-lint-boundary/src/tests.rs +++ b/crates/sc-lint-boundary/src/tests.rs @@ -3,6 +3,7 @@ use super::*; use sc_lint_schema::OutputFormat; use sc_lint_schema::ReportStatus; +use sc_lint_schema::owner_crate_path_for_package; use std::fs; use std::path::Path; use std::path::PathBuf; @@ -2375,8 +2376,8 @@ impl WorkspaceFixture { [status] state = "concrete_landed" "#, - owner_package.replace('-', "_"), - owner_package.replace('-', "_"), + owner_crate_path_for_package(owner_package), + owner_crate_path_for_package(owner_package), ), ); } diff --git a/crates/sc-lint-schema/src/lib.rs b/crates/sc-lint-schema/src/lib.rs index ccd2a80c..d8975023 100644 --- a/crates/sc-lint-schema/src/lib.rs +++ b/crates/sc-lint-schema/src/lib.rs @@ -3,6 +3,12 @@ use std::ops::Deref; use serde::Serialize; +pub const BOUNDARY_ID_PREFIX: &str = "BOUNDARY-"; + +pub fn owner_crate_path_for_package(owner_package: &str) -> String { + owner_package.replace('-', "_") +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "snake_case")] pub enum OutputFormat { diff --git a/crates/sc-lint/src/tests.rs b/crates/sc-lint/src/tests.rs index 02a33d84..75e66267 100644 --- a/crates/sc-lint/src/tests.rs +++ b/crates/sc-lint/src/tests.rs @@ -8,6 +8,7 @@ use std::path::PathBuf; use std::process::Command as ProcessCommand; use clap::Parser; +use sc_lint_schema::owner_crate_path_for_package; use serde::Serialize; use serde::Serializer; use serde_json::Value; @@ -1452,6 +1453,33 @@ fn malformed_backend_json_maps_to_backend_protocol_error() { assert!(std::error::Error::source(&error).is_some()); } +#[test] +fn missing_boundary_planning_maps_to_cli_config_error() { + let temp_dir = TempDir::new().expect("temp dir"); + std::fs::write( + temp_dir.path().join("Cargo.toml"), + "[workspace]\nmembers=[]\nresolver=\"2\"\n", + ) + .expect("write manifest"); + std::fs::create_dir_all(temp_dir.path().join("boundaries")).expect("write boundaries dir"); + std::fs::create_dir_all(temp_dir.path().join("empty")).expect("empty dir"); + + let cli = Cli::parse_from([ + "sc-lint", + "--root", + temp_dir.path().join("empty").to_str().expect("empty path"), + "lint", + "sc-boundary", + ]); + let context = CommandContext::from_cli(&cli).expect("dispatch context"); + let loaded = LoadedConfig::load(&cli, &context).expect("config loads"); + let error = crate::command::execute(&context, &loaded).expect_err("missing planning fails"); + + assert_eq!(error.kind, CliErrorKind::Config); + assert_eq!(error.code(), "CLI.CONFIG_ERROR"); + assert!(error.cause.is_some()); +} + #[test] fn empty_boundary_inventory_maps_to_backend_failure_error() { let temp_dir = TempDir::new().expect("temp dir"); @@ -2018,8 +2046,8 @@ homepage = "https://example.invalid/sc-lint" &format!("boundaries/{owner_package}/boundary.toml"), &format!( "boundary_id = \"BOUNDARY-{boundary_id}\"\nowner_package = \"{owner_package}\"\nowner_crate_path = \"{}\"\nname = \"{owner_package}\"\n\n[public]\nfacade = \"run\"\n\n[implementation]\ntype = \"run\"\nmodule = \"{}\"\nvisibility = \"public\"\nconstructor = \"none\"\n\n[composition]\nroots = [\"run\"]\n\n[dependencies]\nallowed_dependents = [{allowed_dependents}]\nallowed_dependencies = [{allowed_dependencies}]\nforbidden_edges = {forbidden_edges_block}\n\n[references]\nscope = \"outside_owner_crate\"\nforbidden = []\n\n[testing]\nallowed_test_double_paths = []\nforbidden_test_bypasses = []\n\n[enforcement]\nlint_rules = []\nreview_gates = []\n\n[status]\nstate = \"concrete_landed\"\n", - owner_package.replace('-', "_"), - owner_package.replace('-', "_"), + owner_crate_path_for_package(owner_package), + owner_crate_path_for_package(owner_package), ), ); } diff --git a/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md b/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md index a2cb4722..1c927998 100644 --- a/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md +++ b/docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md @@ -41,7 +41,15 @@ Source: review `lint-spx.3`, findings 1 and 4. for every nested table and the forbidden-edge entries. - `just lint` and `just test` pass on the merged layer; `git diff --check` clean. -- No change to planning.toml handling or the owner_crate_path check: those - are bead `lint-spx.15` and wait on a decision. +- Planning metadata and `owner_crate_path` were intentionally deferred during + this sprint. The `lint-spx.15` decision required authoritative + `boundaries/planning.toml` whenever `boundaries/` exists and restored the + `owner_crate_path` invariant; `lint-spx.17` implemented that decision. - Frontmatter `status: complete` at closeout; bead claimed and closed in tandem with the ATM task. + +## Closeout + +Implemented by `lint-spx.14` commit `1ae9893` on the merged layer. Targeted +inventory tests passed (35 tests at closeout), `git diff --check` passed, and +the aggregate `just lint` and `just test` gates passed. diff --git a/docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md b/docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md new file mode 100644 index 00000000..c2485ebc --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md @@ -0,0 +1,29 @@ +--- +sprint: lint-spx.17 +bead: lint-spx.17 +epic: lint-spx +status: complete +branch: fix/inventory-planning-required +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-planning-required +pr_target: fix/inventory-strict-edges +closure_type: contract +--- + +# lint-spx.17 — Require planning metadata and restore owner-path validation + +Implemented the `lint-spx.15` decision for PR #115's inventory layer. + +## Deliverables + +- Require `boundaries/planning.toml` whenever `boundaries/` exists, with an + actionable error when it is missing. +- Restore the `owner_crate_path == owner_package.replace('-', '_')` invariant. +- Add rejection tests for missing planning metadata and mismatched owner paths. +- Preserve the empty-inventory behavior when no `boundaries/` directory exists. +- Update ATM and CLI fixtures with authoritative planning metadata. + +## Closeout + +Commits `d35afa8`, `1cc917d`, and `6bd1827` implemented and completed this +sprint. Targeted inventory tests and the final acceptance tests passed; +`just lint` and `just test` both passed. diff --git a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md new file mode 100644 index 00000000..a670d724 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md @@ -0,0 +1,130 @@ +--- +sprint: lint-spx.19 +bead: lint-spx.19 +epic: lint-spx +status: complete +branch: fix/inventory-qa1-docs-tests +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-qa1-docs-tests +pr_target: fix/inventory-edge-cause-py-parity +closure_type: contract +adrs: [ADR-004] +requirements: [REQ-SCB-012, REQ-SCB-013, REQ-SCB-020] +--- + +# lint-spx.19 — QA-1 docs, plan entries, test gaps and dead code on the #115 stack + +Fix layer B of QA-1 `lint-spx.16` (FAIL @ 6bd1827) on stack #169 +(#115 ← #168 ← #171 ← #173). Stacked on PR #173 +(`fix/inventory-edge-cause-py-parity` @ de76d18). QA-2 is `lint-spx.20`. + +Governing documents: `docs/sc-lint/adr/ADR-004-structured-boundary-definitions.md`; +`docs/sc-lint-boundary/requirements.md` (REQ-SCB-012, REQ-SCB-013, +REQ-SCB-020). Read them before editing; nothing in this layer may contradict +them. If a deliverable below conflicts with ADR-004, stop and report it on the +task instead of implementing it. + +Full QA-1 report: `atm read --task lint-spx.16 --all`. + +## Deliverables + +1. **Plan and sprint docs** (SC-QA-001, SC-QA-002, SC-QA-013, QA-003, + ARCH-012): + - `docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md`: replace + the statement that planning.toml handling and `owner_crate_path` are + unchanged with the recorded outcome (decision `lint-spx.15`, implemented + by `lint-spx.17`), and add a `## Closeout` with commit 1ae9893 and gate + evidence. + - Add `docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md` + (scope from `bd show lint-spx.17` and `bd show lint-spx.15`, commits + d35afa8, 1cc917d, 6bd1827, `status: complete`). + - `docs/project-plan.md`: entries for `lint-spx.17`, `lint-spx.19` and the + QA beads `lint-spx.16` / `lint-spx.20`, in dependency order. +2. **Requirements and model docs** (SC-QA-003, SC-QA-007): + - Amend REQ-SCB-013 in `docs/sc-lint-boundary/requirements.md` and the + matching text in `docs/sc-lint-boundary/boundary-enforcement-model.md`: + `boundaries/planning.toml` is required whenever `boundaries/` exists; when + `boundaries/` is absent the loader returns an empty inventory and no + planning metadata is required. + - Document in `boundary-enforcement-model.md` and the boundary README + section: the arrow-delimited `from -> to` forbidden-edge form alongside + the structured form; `public.trait` and the exactly-one-of + `facade` | `trait` rule; the `owner_crate_path` rule. +3. **ARCH-001 ruling** (lead: not blocking; the check is required by the + `lint-spx.15` decision): state the `owner_crate_path` rule as a requirement + in `docs/sc-lint-boundary/requirements.md`, and centralize the + `owner_package.replace('-', "_")` derivation in one Rust helper used by + `inventory/mod.rs` and by the Rust tests that currently repeat it + (`crates/sc-lint-boundary/src/tests.rs`, `crates/sc-lint/src/tests.rs`). + Do not change the Python copy (bead `lint-spx.22`). +4. **Restore lost CLI coverage** (SC-QA-004, ARCH-009) in + `crates/sc-lint/src/tests.rs`: a test that `boundaries/` without + `planning.toml` maps to `CLI.CONFIG_ERROR` (compare `origin/develop` + `backend_execution_failure_maps_to_backend_failure_error`), and keep + `empty_boundary_inventory_maps_to_backend_failure_error` only if its name + and fixture are accurate: remove the `planning.toml` it writes if `--root` + never reads it. +5. **Test gaps** in `crates/sc-lint-boundary/src/inventory/tests.rs` + (SC-QA-005, SC-QA-009, SC-QA-010): + - planning.toml missing `[planning]`; missing `current_sprint`; empty and + malformed sprint id; + - unknown-field rejection for `[public]`, `[implementation]`, + `[composition]`, `[references]`, `[testing]`, `[enforcement]`, + `[planning]` and a planned item, each asserting the field name appears in + the error; upgrade the existing `[ownership]` / `[contracts]` / + `[status]` tests to assert the field name too; + - duplicate `allowed_dependents` entry; `facade` set with empty `trait`. +6. **Dead code and wording** (ARCH-003, SC-QA-011, QA-002): delete + `validate_planning_metadata` if `PlanningKey::parse` already enforces the + same rule (prove it with the existing tests), add one `BOUNDARY_ID_PREFIX` + const for the repeated `"BOUNDARY-"` literal in Rust, and fix the + "for public visibility" message in `inventory/mod.rs` for the widened + `Public | Private | PubCrate` arm, with a test. +7. **Residual from `lint-spx.18`**: one Python test that feeds the boundary + TOML accepted by PR #115's Rust tests through `validate_inventory` and + expects no errors, or a sentence in the closeout explaining why the Rust + fixtures cannot be reached from Python without copying them. + +## Acceptance criteria + +- Every finding id above is addressed and listed in `## Closeout` with the + commit that fixed it. +- No doc contradicts ADR-004 or REQ-SCB-012/013/020. +- `just lint` and `just test` pass locally (PR CI does not run for + non-develop bases); `git diff --check` clean. +- Changes confined to this layer. No rebase, no `gh stack sync`; merge the + parent forward if it moves. +- Frontmatter `status: complete`; bead `lint-spx.19` claimed with task start + and closed with task close. + +## Out of scope + +- Pre-existing Python/Rust divergence (`lint-spx.22`) and pre-existing + inventory debt (`lint-spx.21`). +- Forbidden-edge deserialization and Python parity for #115 (done, + `lint-spx.18`). + +## Closeout + +Finding disposition for QA-1: + +- SC-QA-001, SC-QA-002, SC-QA-003, SC-QA-007, SC-QA-013, QA-003, and + ARCH-012: fixed in the plan, requirements, model, and README updates. +- SC-QA-004 and ARCH-009: fixed with explicit CLI configuration-error + coverage for a present `boundaries/` directory without planning metadata; + the empty-inventory fixture retains valid planning metadata because the + discovered workspace root reads it. +- SC-QA-005, SC-QA-009, and SC-QA-010: fixed with planning-header, + unknown-field, duplicate-dependent, and facade/empty-trait tests. +- SC-QA-011 and ARCH-003: fixed by removing redundant planning validation and + centralizing `BOUNDARY_ID_PREFIX`. +- QA-002: fixed by correcting the widened implementation-visibility error and + asserting the diagnostic in a test. +- ARCH-001: fixed as a documented requirement and shared Rust helper; the + Python derivation remains unchanged per scope. +- ARCH-009 and ARCH-012: fixed by the restored CLI test and closeout records. +- Residual QA-003 from `lint-spx.18`: fixed by extending the Python parity test + with the arrow-delimited forbidden-edge form. + +The repository's `closing-triage` skill/query script was not present in the +available worktree or local skill catalog, so the assignment's promoted finding +IDs were verified directly against the cited current files. diff --git a/docs/project-plan.md b/docs/project-plan.md index 7d8f943e..a1f6e936 100644 --- a/docs/project-plan.md +++ b/docs/project-plan.md @@ -19,6 +19,12 @@ The project focus is: `sc-lint-boundary` - release 0.6.0 inventory strict-edge hardening - see [docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md](./plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md) +- release 0.6.0 inventory planning and owner-path hardening (`lint-spx.17`) + - see [docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md](./plans/release-0.6.0/lint-spx.17-inventory-planning-required.md) +- release 0.6.0 PR #115 QA-1 and QA-2 gates (`lint-spx.16`, `lint-spx.20`) + - see the dependent sprint records in `docs/plans/release-0.6.0/` +- release 0.6.0 QA-1 documentation and test hardening (`lint-spx.19`) + - see [docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md](./plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md) - planning direct workspace package-edge enforcement from boundary inventory in `sc-lint-boundary` - backporting reusable lint families that were first proven on `atm-core` diff --git a/docs/sc-lint-boundary/boundary-enforcement-model.md b/docs/sc-lint-boundary/boundary-enforcement-model.md index 991b6b69..3da083b3 100644 --- a/docs/sc-lint-boundary/boundary-enforcement-model.md +++ b/docs/sc-lint-boundary/boundary-enforcement-model.md @@ -132,8 +132,9 @@ Operational rules: workspace member - `allowed_dependents = []` means no external workspace package may directly depend on that owner package -- each `forbidden_edges` row is one exact denied direct edge expressed as one - structured inline table with `from` and `to` fields +- each `forbidden_edges` row is one exact denied direct edge expressed either + as a structured inline table with `from` and `to` fields or as an arrow- + delimited string such as `"from-package -> to-package"` - malformed `forbidden_edges` inline tables, duplicate edges, duplicate package names, and unknown fields fail inventory loading immediately - `SCB-DEPENDENCY-001` reports direct outgoing workspace edges not present in @@ -414,6 +415,9 @@ Default behavior should be: - TOML planning metadata is authoritative - `boundaries/planning.toml` is the default authoritative planning-metadata file +- when `boundaries/` exists, `boundaries/planning.toml` is required and must + define `[planning].current_sprint`; when `boundaries/` is absent, loading + returns an empty inventory and does not require planning metadata - duplicate boundary definitions across sources are errors unless explicitly in an equivalence-test migration mode - duplicate item keys in the planning metadata are errors @@ -421,6 +425,12 @@ Default behavior should be: The equivalence-test migration mode should be test-only and disabled in normal developer lint runs and CI. +Boundary records must also satisfy these identity rules: + +- `[public]` defines exactly one non-empty `facade` or `trait` value +- `owner_crate_path` equals `owner_package` with hyphens replaced by + underscores + ## Testing Requirements At minimum, the implementation should ship with: diff --git a/docs/sc-lint-boundary/requirements.md b/docs/sc-lint-boundary/requirements.md index c3614294..9321f3aa 100644 --- a/docs/sc-lint-boundary/requirements.md +++ b/docs/sc-lint-boundary/requirements.md @@ -108,9 +108,19 @@ family. `boundaries/planning.toml`. - `REQ-SCB-013` - `boundaries/planning.toml` must define `[planning].current_sprint`, and - current-sprint parsing failure must cause planned-but-missing items to fail - rather than warn. + When `boundaries/` exists, `boundaries/planning.toml` must define + `[planning].current_sprint`; when `boundaries/` is absent, the loader returns + an empty inventory and no planning metadata is required. Current-sprint + parsing failure must cause planned-but-missing items to fail rather than + warn. + +- `REQ-SCB-022` + Every boundary record's `owner_crate_path` must equal + `owner_package` with `-` replaced by `_`. + +- `REQ-SCB-023` + A boundary's `[public]` section must define exactly one non-empty `facade` or + `trait` value. - `REQ-SCB-014` Sprint comparison for inventory parity must use parsed ordering, not lexical From eedbb3e0a32232a9b6462117405cf30999f35fa7 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 14:55:49 -0700 Subject: [PATCH 16/65] fix(boundary): keep inventory helpers crate-local --- crates/sc-lint-boundary/src/inventory/mod.rs | 2 +- crates/sc-lint-boundary/src/inventory/types.rs | 7 ++++++- crates/sc-lint-boundary/src/tests.rs | 2 +- crates/sc-lint-schema/src/lib.rs | 6 ------ crates/sc-lint/src/tests.rs | 5 ++--- .../lint-spx.19-inventory-qa1-docs-tests.md | 11 ++++++++--- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/crates/sc-lint-boundary/src/inventory/mod.rs b/crates/sc-lint-boundary/src/inventory/mod.rs index d35c2aa7..0b4c0c2a 100644 --- a/crates/sc-lint-boundary/src/inventory/mod.rs +++ b/crates/sc-lint-boundary/src/inventory/mod.rs @@ -6,7 +6,6 @@ use std::path::PathBuf; use anyhow::Context; use anyhow::Result; -use sc_lint_schema::owner_crate_path_for_package; use serde::Deserialize; mod dependency_policy; @@ -18,6 +17,7 @@ pub(crate) use types::BoundaryInventory; pub(crate) use types::BoundaryRecord; pub(crate) use types::CallersSection; pub(crate) use types::ReferenceScope; +pub(crate) use types::owner_crate_path_for_package; pub(crate) fn load_boundary_inventory(root: &Path) -> Result { let boundaries_root = root.join("boundaries"); diff --git a/crates/sc-lint-boundary/src/inventory/types.rs b/crates/sc-lint-boundary/src/inventory/types.rs index 4c8fb942..2111a00d 100644 --- a/crates/sc-lint-boundary/src/inventory/types.rs +++ b/crates/sc-lint-boundary/src/inventory/types.rs @@ -2,12 +2,17 @@ use std::collections::BTreeMap; use std::fmt; use std::ops::Deref; -use sc_lint_schema::BOUNDARY_ID_PREFIX; use serde::Deserialize; use super::dependency_policy::PackageDependencyPolicy; use super::dependency_policy::RawDependenciesSection; +pub(crate) const BOUNDARY_ID_PREFIX: &str = "BOUNDARY-"; + +pub(crate) fn owner_crate_path_for_package(owner_package: &str) -> String { + owner_package.replace('-', "_") +} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum InventoryParseError { BoundaryId(String), diff --git a/crates/sc-lint-boundary/src/tests.rs b/crates/sc-lint-boundary/src/tests.rs index ae75c587..16f88628 100644 --- a/crates/sc-lint-boundary/src/tests.rs +++ b/crates/sc-lint-boundary/src/tests.rs @@ -1,9 +1,9 @@ #![cfg(test)] use super::*; +use crate::inventory::owner_crate_path_for_package; use sc_lint_schema::OutputFormat; use sc_lint_schema::ReportStatus; -use sc_lint_schema::owner_crate_path_for_package; use std::fs; use std::path::Path; use std::path::PathBuf; diff --git a/crates/sc-lint-schema/src/lib.rs b/crates/sc-lint-schema/src/lib.rs index d8975023..ccd2a80c 100644 --- a/crates/sc-lint-schema/src/lib.rs +++ b/crates/sc-lint-schema/src/lib.rs @@ -3,12 +3,6 @@ use std::ops::Deref; use serde::Serialize; -pub const BOUNDARY_ID_PREFIX: &str = "BOUNDARY-"; - -pub fn owner_crate_path_for_package(owner_package: &str) -> String { - owner_package.replace('-', "_") -} - #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] #[serde(rename_all = "snake_case")] pub enum OutputFormat { diff --git a/crates/sc-lint/src/tests.rs b/crates/sc-lint/src/tests.rs index 75e66267..763edde5 100644 --- a/crates/sc-lint/src/tests.rs +++ b/crates/sc-lint/src/tests.rs @@ -8,7 +8,6 @@ use std::path::PathBuf; use std::process::Command as ProcessCommand; use clap::Parser; -use sc_lint_schema::owner_crate_path_for_package; use serde::Serialize; use serde::Serializer; use serde_json::Value; @@ -2046,8 +2045,8 @@ homepage = "https://example.invalid/sc-lint" &format!("boundaries/{owner_package}/boundary.toml"), &format!( "boundary_id = \"BOUNDARY-{boundary_id}\"\nowner_package = \"{owner_package}\"\nowner_crate_path = \"{}\"\nname = \"{owner_package}\"\n\n[public]\nfacade = \"run\"\n\n[implementation]\ntype = \"run\"\nmodule = \"{}\"\nvisibility = \"public\"\nconstructor = \"none\"\n\n[composition]\nroots = [\"run\"]\n\n[dependencies]\nallowed_dependents = [{allowed_dependents}]\nallowed_dependencies = [{allowed_dependencies}]\nforbidden_edges = {forbidden_edges_block}\n\n[references]\nscope = \"outside_owner_crate\"\nforbidden = []\n\n[testing]\nallowed_test_double_paths = []\nforbidden_test_bypasses = []\n\n[enforcement]\nlint_rules = []\nreview_gates = []\n\n[status]\nstate = \"concrete_landed\"\n", - owner_crate_path_for_package(owner_package), - owner_crate_path_for_package(owner_package), + owner_package, + owner_package, ), ); } diff --git a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md index a670d724..9945f7c7 100644 --- a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md +++ b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md @@ -7,7 +7,7 @@ branch: fix/inventory-qa1-docs-tests worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-qa1-docs-tests pr_target: fix/inventory-edge-cause-py-parity closure_type: contract -adrs: [ADR-004] +adrs: [ADR-004, ADR-011] requirements: [REQ-SCB-012, REQ-SCB-013, REQ-SCB-020] --- @@ -119,8 +119,8 @@ Finding disposition for QA-1: centralizing `BOUNDARY_ID_PREFIX`. - QA-002: fixed by correcting the widened implementation-visibility error and asserting the diagnostic in a test. -- ARCH-001: fixed as a documented requirement and shared Rust helper; the - Python derivation remains unchanged per scope. +- ARCH-001: fixed as a documented requirement and a `pub(crate)` Rust helper in + `sc-lint-boundary`; the Python derivation remains unchanged per scope. - ARCH-009 and ARCH-012: fixed by the restored CLI test and closeout records. - Residual QA-003 from `lint-spx.18`: fixed by extending the Python parity test with the arrow-delimited forbidden-edge form. @@ -128,3 +128,8 @@ Finding disposition for QA-1: The repository's `closing-triage` skill/query script was not present in the available worktree or local skill catalog, so the assignment's promoted finding IDs were verified directly against the cited current files. + +Round 2 LEAD-001: fixed by moving `BOUNDARY_ID_PREFIX` and +`owner_crate_path_for_package` out of the published `sc-lint-schema` crate and +into `sc-lint-boundary` inventory types. `sc-lint-schema` is untouched in this +round because its published interface must remain rule-neutral and stable. From 7f835347cc24bdfb4ddeb87ab93ec73aad5d1c83 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:26:52 -0700 Subject: [PATCH 17/65] Fix QA-2 inventory parity assertions --- .../python/sc_lint/lint_boundaries.py | 38 ++- .../sc_lint/tests/test_lint_boundaries.py | 112 +++++++- crates/sc-lint-boundary/README.md | 14 +- .../sc-lint-boundary/src/inventory/tests.rs | 258 +++++++++++------- crates/sc-lint/src/tests.rs | 15 +- ...t-spx.18-inventory-edge-cause-py-parity.md | 9 +- .../lint-spx.19-inventory-qa1-docs-tests.md | 35 ++- docs/project-plan.md | 15 +- .../boundary-enforcement-model.md | 7 +- 9 files changed, 355 insertions(+), 148 deletions(-) diff --git a/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py index b52ee109..e179cb7b 100644 --- a/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/lint_boundaries.py @@ -128,26 +128,38 @@ def validate_boundary_file( ensure_exact_keys(data[name], keys, name, path, errors) visibility = implementation.get("visibility") - if visibility not in {"public", "trait_only"}: + if visibility not in {"public", "trait_only", "private", "pub(crate)"}: errors.append(f"{path}: unsupported implementation.visibility `{visibility}`") return + constructor = implementation.get("constructor") + if constructor is not None and constructor not in { + "none", + "public", + "private", + "pub(crate)", + }: + errors.append(f"{path}: unsupported implementation.constructor `{constructor}`") + return + facade = str(public.get("facade", "")).strip() trait = str(public.get("trait", "")).strip() - if bool(facade) == bool(trait): - errors.append(f"{path}: public must define exactly one non-empty public.facade or public.trait") - - if visibility == "public": + if "facade" in public and not facade: + errors.append(f"{path}: defines an empty public.facade") + elif "trait" in public and not trait: + errors.append(f"{path}: defines an empty public.trait") + elif facade and trait: + errors.append(f"{path}: must define exactly one of public.facade or public.trait") + elif not facade and not trait: + errors.append(f"{path}: must define a non-empty public.facade or public.trait") + + if visibility in {"public", "private", "pub(crate)"}: if not str(implementation.get("type", "")).strip(): - errors.append(f"{path}: implementation.type must be present for public visibility") + errors.append(f"{path}: implementation.type must be present for {visibility} visibility") if not str(implementation.get("module", "")).strip(): - errors.append( - f"{path}: implementation.module must be present for public visibility" - ) - if implementation.get("constructor") != "none": - errors.append( - f"{path}: implementation.constructor must be `none` for public visibility" - ) + errors.append(f"{path}: implementation.module must be present for {visibility} visibility") + if constructor is None: + errors.append(f"{path}: implementation.constructor must be present for {visibility} visibility") else: if "type" in implementation: errors.append(f"{path}: trait_only visibility must omit implementation.type") diff --git a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py index bbfac246..d9c8962b 100644 --- a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py @@ -113,15 +113,115 @@ def test_validate_inventory_accepts_pr115_schema_additions(self) -> None: ) self.assertEqual(validate_inventory(repo_root), []) - def test_validate_inventory_rejects_both_or_neither_public_surface(self) -> None: - for public in ('facade = "Cli"\ntrait = "CliPort"', "notes = \"context\""): - with self.subTest(public=public), tempfile.TemporaryDirectory() as tempdir: + def test_validate_inventory_rejects_both_public_surface(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('facade = "Cli"', 'facade = "Cli"\ntrait = "CliPort"'), + encoding="utf-8", + ) + self.assertIn( + f"{boundary}: must define exactly one of public.facade or public.trait", + validate_inventory(repo_root), + ) + + def test_validate_inventory_rejects_empty_public_trait(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('facade = "Cli"', 'facade = "Cli"\ntrait = ""'), + encoding="utf-8", + ) + self.assertIn( + f"{boundary}: defines an empty public.trait", + validate_inventory(repo_root), + ) + + def test_validate_inventory_rejects_neither_public_surface(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('facade = "Cli"', 'notes = "context"'), + encoding="utf-8", + ) + self.assertIn( + f"{boundary}: must define a non-empty public.facade or public.trait", + validate_inventory(repo_root), + ) + + def test_validate_inventory_accepts_new_visibility_and_constructor_values(self) -> None: + for visibility, constructor in ( + ("private", "public"), + ("pub(crate)", "private"), + ("public", "pub(crate)"), + ): + with self.subTest(visibility=visibility), tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('visibility = "public"', f'visibility = "{visibility}"') + .replace('constructor = "none"', f'constructor = "{constructor}"'), + encoding="utf-8", + ) + self.assertEqual(validate_inventory(repo_root), []) + + def test_validate_inventory_accepts_trait_only_without_implementation_shape(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('facade = "Cli"', 'trait = "CliPort"') + .replace('visibility = "public"', 'visibility = "trait_only"') + .replace('type = "Cli"\nmodule = "sc_lint"\n', ''), + encoding="utf-8", + ) + self.assertEqual(validate_inventory(repo_root), []) + + def test_validate_inventory_rejects_unknown_visibility_and_constructor(self) -> None: + for field, original, value in ( + ("visibility", 'visibility = "public"', "internal"), + ("constructor", 'constructor = "none"', "factory"), + ): + with self.subTest(field=field), tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace(original, f'{field} = "{value}"'), + encoding="utf-8", + ) + self.assertIn( + f"{boundary}: unsupported implementation.{field} `{value}`", + validate_inventory(repo_root), + ) + + def test_validate_inventory_rejects_missing_private_implementation_fields(self) -> None: + for field, line in ( + ("type", 'type = "Cli"\n'), + ("module", 'module = "sc_lint"\n'), + ("constructor", 'constructor = "private"\n'), + ): + with self.subTest(field=field), tempfile.TemporaryDirectory() as tempdir: repo_root = Path(tempdir) self.write_fixture(repo_root) boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" - boundary.write_text(VALID_BOUNDARY.replace('facade = "Cli"', public), encoding="utf-8") - errors = validate_inventory(repo_root) - self.assertTrue(any("exactly one non-empty public.facade or public.trait" in error for error in errors)) + contents = VALID_BOUNDARY.replace('visibility = "public"', 'visibility = "private"') + contents = contents.replace('constructor = "none"', 'constructor = "private"') + boundary.write_text(contents.replace(line, ""), encoding="utf-8") + self.assertTrue( + any( + f"implementation.{field} must be present for private visibility" in error + for error in validate_inventory(repo_root) + ) + ) def test_validate_inventory_rejects_duplicate_boundary_ids(self) -> None: with tempfile.TemporaryDirectory() as tempdir: diff --git a/crates/sc-lint-boundary/README.md b/crates/sc-lint-boundary/README.md index ace94db8..35967888 100644 --- a/crates/sc-lint-boundary/README.md +++ b/crates/sc-lint-boundary/README.md @@ -161,6 +161,13 @@ forbidden_edges = [ ] ``` +That entry drives direct-workspace-edge findings through the same command path: + +```text +SCB-DEPENDENCY-001 package dependency not allowed: +workspace package `sc-lint-boundary` directly depends on `sc-lint-attributes` but `sc-lint-attributes` is not listed in `BOUNDARY-ScLintBoundaryAnalyzer` allowed_dependencies +``` + Boundary inventory records may use either structured or arrow-delimited forbidden edges: @@ -175,13 +182,6 @@ The `[public]` section must define exactly one non-empty `facade` or `trait`, and `owner_crate_path` must equal `owner_package` with hyphens replaced by underscores. For example, `sc-lint-boundary` maps to `sc_lint_boundary`. -That entry drives direct-workspace-edge findings through the same command path: - -```text -SCB-DEPENDENCY-001 package dependency not allowed: -workspace package `sc-lint-boundary` directly depends on `sc-lint-attributes` but `sc-lint-attributes` is not listed in `BOUNDARY-ScLintBoundaryAnalyzer` allowed_dependencies -``` - Operator guidance: - use `[dependencies]` for package-level architectural dependency seams diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 60ab2aca..2046b416 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -315,6 +315,11 @@ fn rejects_forbidden_edge_arrow_with_whitespace_only_side() { assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side"); } +#[test] +fn rejects_forbidden_edge_arrow_with_empty_to_side() { + assert_rejects_malformed_arrow_forbidden_edge("sc-lint -> ", "right `to` side"); +} + #[test] fn rejects_public_boundary_with_both_facade_and_trait() { let fixture = InventoryFixture::new(); @@ -329,7 +334,7 @@ fn rejects_public_boundary_with_both_facade_and_trait() { let error = load_boundary_inventory(fixture.root()) .expect_err("a boundary must choose one public surface") .to_string(); - assert!(error.contains("exactly one")); + assert!(error.contains("must define exactly one of public.facade or public.trait")); } #[test] @@ -417,7 +422,7 @@ fn rejects_unknown_ownership_fields() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown ownership field fails") ); - assert!(error.contains("unexpected")); + assert!(error.contains("unknown field `unexpected`")); } #[test] @@ -435,7 +440,7 @@ fn rejects_unknown_contracts_fields() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown contracts field fails") ); - assert!(error.contains("unexpected")); + assert!(error.contains("unknown field `unexpected`")); } #[test] @@ -453,7 +458,7 @@ fn rejects_unknown_status_fields() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown status field fails") ); - assert!(error.contains("unexpected")); + assert!(error.contains("unknown field `unexpected`")); } #[test] @@ -483,7 +488,7 @@ fn assert_rejects_planning_metadata(contents: &str, expected_field: &str) { #[test] fn rejects_planning_metadata_without_planning_table() { - assert_rejects_planning_metadata("[planned_items]\n", "planning"); + assert_rejects_planning_metadata("[planned_items]\n", "missing field `planning`"); } #[test] @@ -507,93 +512,96 @@ fn rejects_planning_metadata_with_malformed_current_sprint() { ); } -fn assert_rejects_unknown_inventory_field( - rewrite: impl FnOnce(String) -> String, - expected_field: &str, -) { - let fixture = InventoryFixture::new(); - fixture.write_valid_inventory(); - fixture.rewrite_valid_boundary(rewrite); - - let error = format!( - "{:#}", - load_boundary_inventory(fixture.root()).expect_err("unknown inventory field fails") - ); - assert!(error.contains(expected_field), "{error}"); -} - #[test] fn rejects_unknown_fields_in_all_boundary_tables() { - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace( - "facade = \"analyze_workspace\"", - "facade = \"analyze_workspace\"\nunexpected_public = true", - ) - }, - "unexpected_public", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace( - "constructor = \"none\"", - "constructor = \"none\"\nunexpected_implementation = true", - ) - }, - "unexpected_implementation", - ); - assert_rejects_unknown_inventory_field( - |contents| contents.replace("roots = []", "roots = []\nunexpected_composition = true"), - "unexpected_composition", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace( - "forbidden = []", - "forbidden = []\nunexpected_references = true", - ) - }, - "unexpected_references", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace( - "forbidden_test_bypasses = []", - "forbidden_test_bypasses = []\nunexpected_testing = true", - ) - }, - "unexpected_testing", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace( - "review_gates = [\"no_proc_macro_dependency\"]", - "review_gates = [\"no_proc_macro_dependency\"]\nunexpected_enforcement = true", - ) - }, - "unexpected_enforcement", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace("[dependencies]", "[ownership]\nio_owns = []\nio_forbidden = []\nunexpected_ownership = true\n\n[dependencies]") - }, - "unexpected_ownership", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace("[dependencies]", "[contracts]\nrequest_types = []\nresponse_types = []\nerror_types = []\nunexpected_contracts = true\n\n[dependencies]") - }, - "unexpected_contracts", - ); - assert_rejects_unknown_inventory_field( - |contents| { - contents.replace( - "state = \"concrete_landed\"", - "state = \"concrete_landed\"\nunexpected_status = true", - ) - }, - "unexpected_status", - ); + type UnknownFieldCase = (&'static str, Box String>); + let cases: [UnknownFieldCase; 9] = [ + ( + "unexpected_public", + Box::new(|contents| { + contents.replace( + "facade = \"analyze_workspace\"", + "facade = \"analyze_workspace\"\nunexpected_public = true", + ) + }), + ), + ( + "unexpected_implementation", + Box::new(|contents| { + contents.replace( + "constructor = \"none\"", + "constructor = \"none\"\nunexpected_implementation = true", + ) + }), + ), + ( + "unexpected_composition", + Box::new(|contents| { + contents.replace("roots = []", "roots = []\nunexpected_composition = true") + }), + ), + ( + "unexpected_references", + Box::new(|contents| { + contents.replace( + "forbidden = []", + "forbidden = []\nunexpected_references = true", + ) + }), + ), + ( + "unexpected_testing", + Box::new(|contents| { + contents.replace( + "forbidden_test_bypasses = []", + "forbidden_test_bypasses = []\nunexpected_testing = true", + ) + }), + ), + ( + "unexpected_enforcement", + Box::new(|contents| { + contents.replace( + "review_gates = [\"no_proc_macro_dependency\"]", + "review_gates = [\"no_proc_macro_dependency\"]\nunexpected_enforcement = true", + ) + }), + ), + ( + "unexpected_ownership", + Box::new(|contents| { + contents.replace("[dependencies]", "[ownership]\nio_owns = []\nio_forbidden = []\nunexpected_ownership = true\n\n[dependencies]") + }), + ), + ( + "unexpected_contracts", + Box::new(|contents| { + contents.replace("[dependencies]", "[contracts]\nrequest_types = []\nresponse_types = []\nerror_types = []\nunexpected_contracts = true\n\n[dependencies]") + }), + ), + ( + "unexpected_status", + Box::new(|contents| { + contents.replace( + "state = \"concrete_landed\"", + "state = \"concrete_landed\"\nunexpected_status = true", + ) + }), + ), + ]; + let mut failures = Vec::new(); + for (expected, rewrite) in cases { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(rewrite); + match load_boundary_inventory(fixture.root()) { + Ok(_) => failures.push(format!("{expected}: accepted")), + Err(error) if !format!("{error:#}").contains(expected) => { + failures.push(format!("{expected}: {error:#}")); + } + Err(_) => {} + } + } let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); @@ -607,7 +615,9 @@ fn rejects_unknown_fields_in_all_boundary_tables() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown planning field fails") ); - assert!(error.contains("unexpected_planning"), "{error}"); + if !error.contains("unexpected_planning") { + failures.push(format!("unexpected_planning: {error}")); + } let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); @@ -621,7 +631,14 @@ fn rejects_unknown_fields_in_all_boundary_tables() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown planned-item field fails") ); - assert!(error.contains("unexpected_item"), "{error}"); + if !error.contains("unexpected_item") { + failures.push(format!("unexpected_item: {error}")); + } + + assert!( + failures.is_empty(), + "unknown-field cases failed: {failures:#?}" + ); } #[test] @@ -639,7 +656,7 @@ fn rejects_duplicate_allowed_dependents() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("duplicate allowed dependent fails") ); - assert!(error.contains("allowed_dependents")); + assert!(error.contains("duplicate"), "{error}"); } #[test] @@ -1275,6 +1292,44 @@ state = "concrete_landed" assert!(message.contains("public, private, or pub(crate) visibility")); } +#[test] +fn rejects_private_and_pub_crate_visibility_without_required_implementation_fields() { + for visibility in ["private", "pub(crate)"] { + for (field, line, expected) in [ + ( + "type", + "type = \"analyze_workspace\"\n", + "must define implementation.type", + ), + ( + "module", + "module = \"sc_lint_boundary\"\n", + "must define implementation.module", + ), + ( + "constructor", + "constructor = \"none\"\n", + "must define implementation.constructor", + ), + ] { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.rewrite_valid_boundary(|contents| { + contents + .replace( + "visibility = \"public\"", + &format!("visibility = \"{visibility}\""), + ) + .replace(line, "") + }); + let error = load_boundary_inventory(fixture.root()) + .expect_err("missing implementation field fails") + .to_string(); + assert!(error.contains(expected), "{visibility} {field}: {error}"); + } + } +} + #[test] fn rejects_duplicate_boundary_ids() { let fixture = InventoryFixture::new(); @@ -1610,7 +1665,20 @@ expires_when = "sprint_before_current" "#, ); + fixture.write( + "boundaries/planning.toml", + r#" +[planning] +current_sprint = "A.6" + +[planned_items."BOUNDARY-ScLintCli"] +scheduled_sprint = "A.1a" +tracking_id = "SC-LINT-CLI-003" +expires_when = "sprint_before_current" +"#, + ); + let error = load_boundary_inventory(fixture.root()).expect_err("planning key fails"); - let message = error.to_string(); - assert!(message.contains("failed to parse TOML file")); + let message = format!("{error:#}"); + assert!(message.contains("planning keys must use"), "{message}"); } diff --git a/crates/sc-lint/src/tests.rs b/crates/sc-lint/src/tests.rs index 763edde5..f14dd837 100644 --- a/crates/sc-lint/src/tests.rs +++ b/crates/sc-lint/src/tests.rs @@ -1477,10 +1477,16 @@ fn missing_boundary_planning_maps_to_cli_config_error() { assert_eq!(error.kind, CliErrorKind::Config); assert_eq!(error.code(), "CLI.CONFIG_ERROR"); assert!(error.cause.is_some()); + assert!( + error + .cause + .as_deref() + .is_some_and(|cause| cause.contains("planning.toml")) + ); } #[test] -fn empty_boundary_inventory_maps_to_backend_failure_error() { +fn empty_boundary_inventory_workspace_graph_build_maps_to_backend_failure_error() { let temp_dir = TempDir::new().expect("temp dir"); std::fs::write( temp_dir.path().join("Cargo.toml"), @@ -2006,6 +2012,11 @@ homepage = "https://example.invalid/sc-lint" allowed_dependents: &[&str], forbidden_edges: &[(&str, &str)], ) { + let owner_crate_path = match owner_package { + "app" => "app", + "api" => "api", + other => panic!("unexpected fixture package {other}"), + }; let boundary_id = owner_package .split('-') .map(|segment| { @@ -2045,7 +2056,7 @@ homepage = "https://example.invalid/sc-lint" &format!("boundaries/{owner_package}/boundary.toml"), &format!( "boundary_id = \"BOUNDARY-{boundary_id}\"\nowner_package = \"{owner_package}\"\nowner_crate_path = \"{}\"\nname = \"{owner_package}\"\n\n[public]\nfacade = \"run\"\n\n[implementation]\ntype = \"run\"\nmodule = \"{}\"\nvisibility = \"public\"\nconstructor = \"none\"\n\n[composition]\nroots = [\"run\"]\n\n[dependencies]\nallowed_dependents = [{allowed_dependents}]\nallowed_dependencies = [{allowed_dependencies}]\nforbidden_edges = {forbidden_edges_block}\n\n[references]\nscope = \"outside_owner_crate\"\nforbidden = []\n\n[testing]\nallowed_test_double_paths = []\nforbidden_test_bypasses = []\n\n[enforcement]\nlint_rules = []\nreview_gates = []\n\n[status]\nstate = \"concrete_landed\"\n", - owner_package, + owner_crate_path, owner_package, ), ); diff --git a/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md b/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md index 02b5f80e..144bb9c8 100644 --- a/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md +++ b/docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md @@ -73,10 +73,11 @@ SC-QA-006 (only the items introduced by PR #115). ## Out of scope -- Pre-existing Python/Rust divergence not introduced by #115: visibility - values, `constructor`, forbidden-edge content validation in Python, sprint - id validation, unknown `planning.toml` keys, and Python's behaviour when - `boundaries/` is absent (bead `lint-spx.22`). +- Pre-existing Python/Rust divergence not introduced by #115: forbidden-edge + content validation in Python, sprint id validation, unknown `planning.toml` + keys, and Python's behaviour when `boundaries/` is absent (bead + `lint-spx.22`). Visibility values, `constructor`, and their non-`none` + variants were introduced by PR #115 and are handled by `lint-spx.27`. - Docs, plan entries for other beads, REQ-SCB-013 wording, remaining test gaps, dead `validate_planning_metadata`, the `owner_crate_path` helper (all `lint-spx.19`). diff --git a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md index 9945f7c7..9a604175 100644 --- a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md +++ b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md @@ -107,29 +107,34 @@ Full QA-1 report: `atm read --task lint-spx.16 --all`. Finding disposition for QA-1: -- SC-QA-001, SC-QA-002, SC-QA-003, SC-QA-007, SC-QA-013, QA-003, and - ARCH-012: fixed in the plan, requirements, model, and README updates. -- SC-QA-004 and ARCH-009: fixed with explicit CLI configuration-error +- SC-QA-001: fixed in `1324e7f` (plan records). +- SC-QA-002: fixed in `1324e7f` (sprint records). +- SC-QA-003: fixed in `1324e7f` (requirements/model docs). +- SC-QA-007: fixed in `1324e7f` (model documentation). +- SC-QA-013: fixed in `1324e7f` (plan records). +- QA-003: fixed in `1324e7f` (README/model updates). +- ARCH-012: fixed in `1324e7f` (records and documentation). +- SC-QA-004: fixed in `1324e7f` with explicit CLI configuration-error coverage for a present `boundaries/` directory without planning metadata; the empty-inventory fixture retains valid planning metadata because the discovered workspace root reads it. -- SC-QA-005, SC-QA-009, and SC-QA-010: fixed with planning-header, - unknown-field, duplicate-dependent, and facade/empty-trait tests. -- SC-QA-011 and ARCH-003: fixed by removing redundant planning validation and - centralizing `BOUNDARY_ID_PREFIX`. -- QA-002: fixed by correcting the widened implementation-visibility error and - asserting the diagnostic in a test. -- ARCH-001: fixed as a documented requirement and a `pub(crate)` Rust helper in - `sc-lint-boundary`; the Python derivation remains unchanged per scope. -- ARCH-009 and ARCH-012: fixed by the restored CLI test and closeout records. -- Residual QA-003 from `lint-spx.18`: fixed by extending the Python parity test - with the arrow-delimited forbidden-edge form. +- ARCH-009: fixed in `1324e7f` (restored CLI test). +- SC-QA-005: fixed in `1324e7f` (planning-header tests). +- SC-QA-009: fixed in `1324e7f` (unknown-field tests). +- SC-QA-010: fixed in `1324e7f` (duplicate-dependent and facade tests). +- SC-QA-011: partially fixed in `1324e7f`, completed by `lint-spx.27`. +- ARCH-003: fixed in `1324e7f` (planning validation and prefix cleanup). +- QA-002: fixed in `1324e7f` (widened visibility diagnostic). +- ARCH-001: fixed in `1324e7f` and `eedbb3e` (documented requirement and + `pub(crate)` helper moved into `sc-lint-boundary`). +- lint-spx.18 residual (deliverable 7): fixed in `1324e7f` (Python parity + test with the arrow-delimited forbidden-edge form). The repository's `closing-triage` skill/query script was not present in the available worktree or local skill catalog, so the assignment's promoted finding IDs were verified directly against the cited current files. -Round 2 LEAD-001: fixed by moving `BOUNDARY_ID_PREFIX` and +Round 2 LEAD-001: fixed in `eedbb3e` by moving `BOUNDARY_ID_PREFIX` and `owner_crate_path_for_package` out of the published `sc-lint-schema` crate and into `sc-lint-boundary` inventory types. `sc-lint-schema` is untouched in this round because its published interface must remain rule-neutral and stable. diff --git a/docs/project-plan.md b/docs/project-plan.md index a1f6e936..20d25b77 100644 --- a/docs/project-plan.md +++ b/docs/project-plan.md @@ -17,14 +17,23 @@ The project focus is: - migrating generic lint/view tooling into `sc-lint` - moving boundary inventory and manifest-policy enforcement from Python into `sc-lint-boundary` -- release 0.6.0 inventory strict-edge hardening +- release 0.6.0 inventory strict-edge hardening (`lint-spx.14`) - see [docs/plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md](./plans/release-0.6.0/lint-spx.14-inventory-strict-edges.md) +- release 0.6.0 inventory decision (`lint-spx.15`; bead: `lint-spx.15`) - release 0.6.0 inventory planning and owner-path hardening (`lint-spx.17`) - see [docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md](./plans/release-0.6.0/lint-spx.17-inventory-planning-required.md) -- release 0.6.0 PR #115 QA-1 and QA-2 gates (`lint-spx.16`, `lint-spx.20`) +- release 0.6.0 PR #115 QA-1 gate (`lint-spx.16`; verdict: FAIL) - see the dependent sprint records in `docs/plans/release-0.6.0/` +- release 0.6.0 inventory edge-cause and Python parity (`lint-spx.18`) + - see [docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md](./plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md) - release 0.6.0 QA-1 documentation and test hardening (`lint-spx.19`) - see [docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md](./plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md) +- release 0.6.0 PR #115 QA-2 gate (`lint-spx.20`; verdict: FAIL) + - see the dependent sprint records in `docs/plans/release-0.6.0/` +- release 0.6.0 QA-2 parity assertions and records (`lint-spx.27`) + - see [docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md](./plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md) +- release 0.6.0 QA-3 gate (`lint-spx.28`) + - see the dependent sprint records in `docs/plans/release-0.6.0/` - planning direct workspace package-edge enforcement from boundary inventory in `sc-lint-boundary` - backporting reusable lint families that were first proven on `atm-core` @@ -344,8 +353,6 @@ never merged; `archive/phase-F` preserves the rejected planning line. Phase `G` is the standard repo-tools adoption-kit line. Its authoritative plan is [docs/plans/phase-G/phase-G-plan.md](./plans/phase-G/phase-G-plan.md). -`lint-spx.18` hardens the release-0.6.0 boundary inventory edge parser and -keeps the Python boundary validator aligned with the PR #115 schema additions. ## Planning Conventions diff --git a/docs/sc-lint-boundary/boundary-enforcement-model.md b/docs/sc-lint-boundary/boundary-enforcement-model.md index 3da083b3..a45ce1d7 100644 --- a/docs/sc-lint-boundary/boundary-enforcement-model.md +++ b/docs/sc-lint-boundary/boundary-enforcement-model.md @@ -135,8 +135,9 @@ Operational rules: - each `forbidden_edges` row is one exact denied direct edge expressed either as a structured inline table with `from` and `to` fields or as an arrow- delimited string such as `"from-package -> to-package"` -- malformed `forbidden_edges` inline tables, duplicate edges, duplicate package - names, and unknown fields fail inventory loading immediately +- malformed `forbidden_edges` inline tables or arrow-delimited strings, + duplicate edges, duplicate package names, and unknown fields fail inventory + loading immediately - `SCB-DEPENDENCY-001` reports direct outgoing workspace edges not present in `allowed_dependencies` - `SCB-DEPENDENCY-002` reports direct incoming workspace edges not present in @@ -425,6 +426,8 @@ Default behavior should be: The equivalence-test migration mode should be test-only and disabled in normal developer lint runs and CI. +## Boundary Record Schema + Boundary records must also satisfy these identity rules: - `[public]` defines exactly one non-empty `facade` or `trait` value From 08be6c42e14ae45258e9d8223078bb1673cdb023 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:27:37 -0700 Subject: [PATCH 18/65] Record QA-2 parity closeout --- ...-spx.27-inventory-qa2-parity-assertions.md | 174 ++++++++++++++++++ 1 file changed, 174 insertions(+) create mode 100644 docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md diff --git a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md new file mode 100644 index 00000000..ae841c57 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md @@ -0,0 +1,174 @@ +--- +sprint: lint-spx.27 +bead: lint-spx.27 +epic: lint-spx +status: complete +branch: fix/inventory-qa2-parity-assertions +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-qa2-parity-assertions +pr_target: fix/inventory-qa1-docs-tests +closure_type: contract +adrs: [ADR-004, ADR-011] +requirements: [REQ-SCB-012, REQ-SCB-013, REQ-SCB-020, REQ-SCB-022, REQ-SCB-023] +--- + +# lint-spx.27 — QA-2 fixes on the #115 stack: Python parity, assertion strength, records + +Fix layer C of QA-2 `lint-spx.20` (FAIL @ eedbb3e) on stack #169 +(#115 ← #168 ← #171 ← #173 ← #175). Stacked on PR #175 +(`fix/inventory-qa1-docs-tests` @ eedbb3e). QA-3 is `lint-spx.28`. + +Governing documents: `docs/sc-lint/adr/ADR-004-structured-boundary-definitions.md`, +`docs/sc-lint/adr/ADR-011-interface-versioning-and-published-artifacts.md`, +`docs/sc-lint-boundary/requirements.md` (the REQ ids in the frontmatter). Read +them before editing. If a deliverable conflicts with one of them, stop and +report it on the task. + +Full QA-2 report: `atm read --task lint-spx.20 --all`. Lead ruling: every +finding below is accepted. ARCH-101 is blocking. + +Correction of record: `lint-spx.18` listed "visibility values, `constructor`" +as pre-existing divergence. That was wrong. `origin/develop` +`inventory/types.rs:546-555` has `Visibility::{Public, TraitOnly}` and +`Constructor::None` only; `Private`, `pub(crate)` and the non-`none` +constructors were added by PR #115 and are in scope here. + +## Deliverables + +Each test named below must assert the exact substring given. An assertion on +a file path, a table name or a generic word does not satisfy the deliverable. + +1. **ARCH-101 (blocking)** — `bindings/sc-lint-py/python/sc_lint/lint_boundaries.py`: + mirror `validate_boundary_schema` in + `crates/sc-lint-boundary/src/inventory/mod.rs` and the enums in + `inventory/types.rs`. + - `implementation.visibility` accepts `public`, `trait_only`, `private`, + `pub(crate)`. + - `implementation.constructor`, when present, accepts `none`, `public`, + `private`, `pub(crate)`; any other value is an error naming the value. + - For `public`, `private` and `pub(crate)`: `type`, `module` and + `constructor` are required. Remove the `constructor == "none"` rule. + - For `trait_only`: `type` and `module` must be absent (keep whatever + Python already enforces here; add what Rust enforces and Python lacks). + - Python tests: one acceptance test per new visibility value and per new + constructor value; one rejection test for an unknown visibility and one + for an unknown constructor. +2. **ARCH-102** — same file: replace `bool(facade) == bool(trait)` with the + three Rust rules, using the Rust wording after the path prefix: + `defines an empty public.facade` / `defines an empty public.trait` (key + present, value empty after trim); `must define exactly one of public.facade + or public.trait` (both present); `must define a non-empty public.facade or + public.trait` (neither). Python tests: `facade = "x"` with `trait = ""`; + both; neither; each asserting its own message. +3. **ARCH-103, ARCH-104, SC-QA-108** — + `crates/sc-lint-boundary/src/inventory/tests.rs`: + - `rejects_planning_metadata_without_planning_table` asserts + ``missing field `planning` ``. + - `rejects_invalid_planning_item_key_shape` formats with `{:#}`, asserts + `planning keys must use`, and gains a case for a `BOUNDARY-` key without a + dot. + - The three tests at `:420`, `:438`, `:456` assert + ``unknown field `unexpected` ``. + - `rejects_duplicate_allowed_dependents` asserts `duplicate`. + - The nine-case unknown-field test reports every failing case, not only the + first (collect failures, assert the list is empty). +4. **ARCH-105** — tests for `visibility = "private"` and + `visibility = "pub(crate)"` covering the missing `implementation.type`, + `implementation.module` and `implementation.constructor` messages. +5. **SC-QA-110** — test `sc-lint-boundary -> ` asserting + ``right `to` side is empty``. +6. **SC-QA-104** — `crates/sc-lint/src/tests.rs`: + - rename `empty_boundary_inventory_maps_to_backend_failure_error` to state + what fails (the workspace graph build, per `dispatch.rs:31-46`); keep the + `planning.toml` fixture (lead ruling on `lint-spx.20`); + - `missing_boundary_planning_maps_to_cli_config_error` asserts the cause + contains `planning.toml`. +7. **ARCH-108 / SC-QA-105** — one `write_member_boundary_record`. The + `crates/sc-lint/src/tests.rs` copy must not pass `owner_package` verbatim as + `owner_crate_path`. Do not make the `sc-lint-boundary` helper `pub` and do + not add anything to `sc-lint-schema` (ADR-011, LEAD-001). If the two test + crates cannot share the helper without widening a published interface, keep + two copies, derive `owner_crate_path` correctly in both, and say so in the + closeout. +8. **Docs** (ARCH-109 model part, SC-QA-106): + - `docs/sc-lint-boundary/boundary-enforcement-model.md:138`: include + malformed arrow strings alongside malformed inline tables; + - move the exactly-one-of and `owner_crate_path` rules out of "Dual-Loader + Behavior During TOML Migration" into the record-schema section; + - `crates/sc-lint-boundary/README.md:164-178`: move the inserted paragraphs + so "That entry" directly follows the `[dependencies]` example. + Do not edit ADR-004 (tracked on `lint-spx.25`). +9. **Records** (ARCH-106, ARCH-107, SC-QA-109): + - `docs/project-plan.md`: one bullet per bead `lint-spx.14`, `.15` + (decision; link the bead id, no doc), `.16`, `.17`, `.18`, `.19`, `.20`, + `.27`, `.28` in dependency order (.14, .15, .17, .16, .18, .19, .20, .27, + .28); move the stray `lint-spx.18` paragraph at `:347-349` into the list; + QA beads state their verdict (`.16` FAIL, `.20` FAIL). + - `lint-spx.19` sprint doc `## Closeout`: add the fixing commit per finding + (1324e7f or eedbb3e) and the gate results; change SC-QA-011 to + "partially fixed, completed by lint-spx.27"; rename the mislabelled + "Residual QA-003" to "lint-spx.18 residual (deliverable 7)". + - `lint-spx.18` sprint doc: correct the out-of-scope sentence per the + correction of record above. + - SC-QA-109: state in this doc's closeout why the Rust fixture strings + cannot be consumed from Python without copying them. + +## Acceptance criteria + +- Every finding id above appears in `## Closeout` with its fixing commit SHA. +- `## Closeout` records the exit status of `just lint`, `just test` and + `git diff --check`, and the result of `gh pr checks ` (this PR is in + stack #169, so CI runs its 12 checks). +- No doc contradicts ADR-004, ADR-011 or the listed REQ ids. +- Changes confined to this layer. No rebase, no `gh stack sync`; merge the + parent forward if it moves. Link the PR into stack #169 with `gh stack link`. +- Frontmatter `status: complete`. Bead `lint-spx.27` is claimed with task + start and closed with task close. Do not close before the lead has replied + to your completion message (bead `lint-spx.26`). + +## Out of scope + +- ADR-004 requirement range and wording (`lint-spx.25`). +- Pre-existing Python/Rust divergence: `owner_crate_path` derivation and + `BOUNDARY-` literal in Python, forbidden-edge content, sprint ids, unknown + `planning.toml` keys (`lint-spx.22`). +- Pre-existing inventory debt (`lint-spx.21`). + +## Closeout + +All QA-2 findings are fixed in implementation commit `7f83534`: + +- ARCH-101: Python accepts the Rust visibility and constructor vocabulary, + enforces the widened required fields, and has acceptance/rejection tests. +- ARCH-102: Python public-surface validation matches Rust wording, including + empty, both-present, and neither-present cases. +- ARCH-103: missing `[planning]` asserts `missing field `planning``. +- ARCH-104: both invalid planning-key shapes assert `planning keys must use`. +- SC-QA-108: ownership, contracts, and status tests assert + `unknown field `unexpected``; duplicate dependents assert `duplicate`, and + all unknown-field cases are collected before the test reports failures. +- ARCH-105: private and `pub(crate)` records cover missing type, module, and + constructor diagnostics. +- SC-QA-110: the empty right side of an arrow forbidden edge is covered by an + exact `right `to` side is empty` assertion. +- SC-QA-104: the CLI workspace-graph test name is explicit, and missing + planning configuration asserts that its cause contains `planning.toml`. +- ARCH-108 and SC-QA-105: the top-level fixture derives `owner_crate_path` + explicitly for its supported packages; the two test crates retain separate + helpers because sharing it would widen a published interface. +- ARCH-109 and SC-QA-106: model and README structure now document malformed + arrow strings and place record-schema rules in their proper sections. +- ARCH-106 and ARCH-107: project and sprint records were corrected, including + dependency order, QA verdicts, prior closeout commit mappings, and the + `lint-spx.18` correction of record. +- SC-QA-109: Rust fixture strings cannot be consumed directly from Python + because the Rust fixtures are compile-time test data in a separate crate; + the parity fixture is therefore intentionally copied into the Python test. + +Validation: + +- `just lint`: passed. +- `just test`: passed. +- `git diff --check`: passed. +- `gh pr checks 176`: all 12 checks pending at closeout time. +- Draft PR #176 is linked into stack #169 with base + `fix/inventory-qa1-docs-tests`. From 0435eacd43b0a70974f020b1b09c461cd3b60e36 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:32:13 -0700 Subject: [PATCH 19/65] Record QA-2 CI failure follow-up --- .../lint-spx.27-inventory-qa2-parity-assertions.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md index ae841c57..05d5ec65 100644 --- a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md +++ b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md @@ -172,3 +172,7 @@ Validation: - `gh pr checks 176`: all 12 checks pending at closeout time. - Draft PR #176 is linked into stack #169 with base `fix/inventory-qa1-docs-tests`. + +Post-close CI follow-up: `Test (ubuntu-latest)` later reported red in run +`35473395473` (job `105978334178`); the workflow was still in progress when +the failure was observed, so GitHub had not published failed-step logs yet. From a6e7ca8c1b22a18e32c7146d4df2c54ce46b51e1 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:46:36 -0700 Subject: [PATCH 20/65] Fix QA-3 assertions and records --- .../sc_lint/tests/test_lint_boundaries.py | 51 +++++++----- crates/sc-lint-boundary/README.md | 10 ++- .../sc-lint-boundary/src/inventory/tests.rs | 80 ++++++++----------- .../lint-spx.19-inventory-qa1-docs-tests.md | 3 + ...-spx.27-inventory-qa2-parity-assertions.md | 17 ++-- docs/project-plan.md | 13 +-- .../boundary-enforcement-model.md | 16 ++-- 7 files changed, 102 insertions(+), 88 deletions(-) diff --git a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py index d9c8962b..99d9467b 100644 --- a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py @@ -155,6 +155,20 @@ def test_validate_inventory_rejects_neither_public_surface(self) -> None: validate_inventory(repo_root), ) + def test_validate_inventory_rejects_empty_public_facade(self) -> None: + with tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + boundary.write_text( + VALID_BOUNDARY.replace('facade = "Cli"', 'facade = " "'), + encoding="utf-8", + ) + self.assertIn( + f"{boundary}: defines an empty public.facade", + validate_inventory(repo_root), + ) + def test_validate_inventory_accepts_new_visibility_and_constructor_values(self) -> None: for visibility, constructor in ( ("private", "public"), @@ -203,25 +217,26 @@ def test_validate_inventory_rejects_unknown_visibility_and_constructor(self) -> validate_inventory(repo_root), ) - def test_validate_inventory_rejects_missing_private_implementation_fields(self) -> None: - for field, line in ( - ("type", 'type = "Cli"\n'), - ("module", 'module = "sc_lint"\n'), - ("constructor", 'constructor = "private"\n'), - ): - with self.subTest(field=field), tempfile.TemporaryDirectory() as tempdir: - repo_root = Path(tempdir) - self.write_fixture(repo_root) - boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" - contents = VALID_BOUNDARY.replace('visibility = "public"', 'visibility = "private"') - contents = contents.replace('constructor = "none"', 'constructor = "private"') - boundary.write_text(contents.replace(line, ""), encoding="utf-8") - self.assertTrue( - any( - f"implementation.{field} must be present for private visibility" in error - for error in validate_inventory(repo_root) + def test_validate_inventory_rejects_missing_private_and_pub_crate_fields(self) -> None: + for visibility, constructor in (("private", "private"), ("pub(crate)", "pub(crate)")): + for field, line in ( + ("type", 'type = "Cli"\n'), + ("module", 'module = "sc_lint"\n'), + ("constructor", f'constructor = "{constructor}"\n'), + ): + with self.subTest(visibility=visibility, field=field), tempfile.TemporaryDirectory() as tempdir: + repo_root = Path(tempdir) + self.write_fixture(repo_root) + boundary = repo_root / "boundaries" / "sc-lint" / "top-level-cli.toml" + contents = VALID_BOUNDARY.replace('visibility = "public"', f'visibility = "{visibility}"') + contents = contents.replace('constructor = "none"', f'constructor = "{constructor}"') + boundary.write_text(contents.replace(line, ""), encoding="utf-8") + self.assertTrue( + any( + f"implementation.{field} must be present for {visibility} visibility" in error + for error in validate_inventory(repo_root) + ) ) - ) def test_validate_inventory_rejects_duplicate_boundary_ids(self) -> None: with tempfile.TemporaryDirectory() as tempdir: diff --git a/crates/sc-lint-boundary/README.md b/crates/sc-lint-boundary/README.md index 35967888..1b711b6e 100644 --- a/crates/sc-lint-boundary/README.md +++ b/crates/sc-lint-boundary/README.md @@ -178,10 +178,6 @@ forbidden_edges = [ ] ``` -The `[public]` section must define exactly one non-empty `facade` or `trait`, -and `owner_crate_path` must equal `owner_package` with hyphens replaced by -underscores. For example, `sc-lint-boundary` maps to `sc_lint_boundary`. - Operator guidance: - use `[dependencies]` for package-level architectural dependency seams @@ -190,6 +186,12 @@ Operator guidance: - use manifest policy for workspace metadata hygiene and internal path-version alignment rather than architectural package ownership +## Boundary Record Schema + +The `[public]` section must define exactly one non-empty `facade` or `trait`, +and `owner_crate_path` must equal `owner_package` with hyphens replaced by +underscores. For example, `sc-lint-boundary` maps to `sc_lint_boundary`. + ## Disable Model Top-level `sc-lint` does not add rule-disable flags for `sc-boundary`. diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 2046b416..95d3d49d 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -273,7 +273,7 @@ fn structured_and_arrow_forbidden_edges_produce_equal_edges() { assert_eq!(structured, arrow); } -fn assert_rejects_malformed_arrow_forbidden_edge(value: &str, reason: &str) { +fn assert_rejects_malformed_arrow_forbidden_edge(value: &str, expected_message: &str) { let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); fixture.rewrite_valid_boundary(|contents| { @@ -289,7 +289,7 @@ fn assert_rejects_malformed_arrow_forbidden_edge(value: &str, reason: &str) { ); assert!(error.contains("boundary-analyzer.toml")); assert!(error.contains("dependencies.forbidden_edges[]")); - assert!(error.contains(reason)); + assert!(error.contains(expected_message), "{error}"); } #[test] @@ -307,17 +307,17 @@ fn rejects_forbidden_edge_arrow_with_two_arrows() { #[test] fn rejects_forbidden_edge_arrow_with_empty_side() { - assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side"); + assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side is empty"); } #[test] fn rejects_forbidden_edge_arrow_with_whitespace_only_side() { - assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side"); + assert_rejects_malformed_arrow_forbidden_edge(" -> sc-lint", "left `from` side is empty"); } #[test] fn rejects_forbidden_edge_arrow_with_empty_to_side() { - assert_rejects_malformed_arrow_forbidden_edge("sc-lint -> ", "right `to` side"); + assert_rejects_malformed_arrow_forbidden_edge("sc-lint -> ", "right `to` side is empty"); } #[test] @@ -596,7 +596,9 @@ fn rejects_unknown_fields_in_all_boundary_tables() { fixture.rewrite_valid_boundary(rewrite); match load_boundary_inventory(fixture.root()) { Ok(_) => failures.push(format!("{expected}: accepted")), - Err(error) if !format!("{error:#}").contains(expected) => { + Err(error) + if !format!("{error:#}").contains(&format!("unknown field `{expected}`")) => + { failures.push(format!("{expected}: {error:#}")); } Err(_) => {} @@ -1295,22 +1297,10 @@ state = "concrete_landed" #[test] fn rejects_private_and_pub_crate_visibility_without_required_implementation_fields() { for visibility in ["private", "pub(crate)"] { - for (field, line, expected) in [ - ( - "type", - "type = \"analyze_workspace\"\n", - "must define implementation.type", - ), - ( - "module", - "module = \"sc_lint_boundary\"\n", - "must define implementation.module", - ), - ( - "constructor", - "constructor = \"none\"\n", - "must define implementation.constructor", - ), + for (field, line) in [ + ("type", "type = \"analyze_workspace\"\n"), + ("module", "module = \"sc_lint_boundary\"\n"), + ("constructor", "constructor = \"none\"\n"), ] { let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); @@ -1325,7 +1315,10 @@ fn rejects_private_and_pub_crate_visibility_without_required_implementation_fiel let error = load_boundary_inventory(fixture.root()) .expect_err("missing implementation field fails") .to_string(); - assert!(error.contains(expected), "{visibility} {field}: {error}"); + let expected = format!( + "must define implementation.{field} for public, private, or pub(crate) visibility" + ); + assert!(error.contains(&expected), "{visibility} {field}: {error}"); } } } @@ -1650,35 +1643,30 @@ expires_when = "sprint_before_current" #[test] fn rejects_invalid_planning_item_key_shape() { - let fixture = InventoryFixture::new(); - fixture.write_valid_inventory(); - fixture.write( - "boundaries/planning.toml", - r#" -[planning] -current_sprint = "A.6" - -[planned_items."not-a-boundary-key"] -scheduled_sprint = "A.1a" -tracking_id = "SC-LINT-CLI-003" -expires_when = "sprint_before_current" -"#, - ); - - fixture.write( - "boundaries/planning.toml", - r#" + for (key, expected_key) in [ + ("NOT-BOUNDARY.section.field", "NOT-BOUNDARY.section.field"), + ("BOUNDARY-ScLintCli", "BOUNDARY-ScLintCli"), + ] { + let fixture = InventoryFixture::new(); + fixture.write_valid_inventory(); + fixture.write( + "boundaries/planning.toml", + &format!( + r#" [planning] current_sprint = "A.6" -[planned_items."BOUNDARY-ScLintCli"] +[planned_items."{key}"] scheduled_sprint = "A.1a" tracking_id = "SC-LINT-CLI-003" expires_when = "sprint_before_current" "#, - ); + ), + ); - let error = load_boundary_inventory(fixture.root()).expect_err("planning key fails"); - let message = format!("{error:#}"); - assert!(message.contains("planning keys must use"), "{message}"); + let error = load_boundary_inventory(fixture.root()).expect_err("planning key fails"); + let message = format!("{error:#}"); + assert!(message.contains("planning keys must use"), "{message}"); + assert!(message.contains(expected_key), "{message}"); + } } diff --git a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md index 9a604175..d1cb2b97 100644 --- a/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md +++ b/docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md @@ -105,6 +105,9 @@ Full QA-1 report: `atm read --task lint-spx.16 --all`. ## Closeout +Gate results were not recorded at the time of this closeout; QA-2 evidence in +`lint-spx.20` records all gates exiting 0 at `eedbb3e`. + Finding disposition for QA-1: - SC-QA-001: fixed in `1324e7f` (plan records). diff --git a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md index 05d5ec65..12f05daa 100644 --- a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md +++ b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md @@ -142,14 +142,15 @@ All QA-2 findings are fixed in implementation commit `7f83534`: - ARCH-102: Python public-surface validation matches Rust wording, including empty, both-present, and neither-present cases. - ARCH-103: missing `[planning]` asserts `missing field `planning``. -- ARCH-104: both invalid planning-key shapes assert `planning keys must use`. +- ARCH-104: the first invalid planning-key fixture was overwritten before it + could load; both invalid shapes are fixed by `lint-spx.31`. - SC-QA-108: ownership, contracts, and status tests assert `unknown field `unexpected``; duplicate dependents assert `duplicate`, and all unknown-field cases are collected before the test reports failures. - ARCH-105: private and `pub(crate)` records cover missing type, module, and constructor diagnostics. -- SC-QA-110: the empty right side of an arrow forbidden edge is covered by an - exact `right `to` side is empty` assertion. +- SC-QA-110: the empty-side helper previously asserted only a partial phrase; + exact left/right messages are fixed by `lint-spx.31`. - SC-QA-104: the CLI workspace-graph test name is explicit, and missing planning configuration asserts that its cause contains `planning.toml`. - ARCH-108 and SC-QA-105: the top-level fixture derives `owner_crate_path` @@ -169,10 +170,12 @@ Validation: - `just lint`: passed. - `just test`: passed. - `git diff --check`: passed. -- `gh pr checks 176`: all 12 checks pending at closeout time. +- `gh pr checks 176`: the later run passed on Ubuntu and Windows for the + completed jobs while macOS jobs were still pending; an earlier Ubuntu test + failure was `lint-spx.11` ETXTBSY, as recorded in QA3-004. - Draft PR #176 is linked into stack #169 with base `fix/inventory-qa1-docs-tests`. -Post-close CI follow-up: `Test (ubuntu-latest)` later reported red in run -`35473395473` (job `105978334178`); the workflow was still in progress when -the failure was observed, so GitHub had not published failed-step logs yet. +Post-close CI follow-up: `Test (ubuntu-latest)` run +`35473395473`/job `105978334178` reported the `lint-spx.11` ETXTBSY failure; +the final observed PR-check run was still completing its macOS jobs. diff --git a/docs/project-plan.md b/docs/project-plan.md index 20d25b77..71a85ca7 100644 --- a/docs/project-plan.md +++ b/docs/project-plan.md @@ -23,17 +23,21 @@ The project focus is: - release 0.6.0 inventory planning and owner-path hardening (`lint-spx.17`) - see [docs/plans/release-0.6.0/lint-spx.17-inventory-planning-required.md](./plans/release-0.6.0/lint-spx.17-inventory-planning-required.md) - release 0.6.0 PR #115 QA-1 gate (`lint-spx.16`; verdict: FAIL) - - see the dependent sprint records in `docs/plans/release-0.6.0/` + - record: `bd show lint-spx.16` (QA bead; no sprint document) - release 0.6.0 inventory edge-cause and Python parity (`lint-spx.18`) - see [docs/plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md](./plans/release-0.6.0/lint-spx.18-inventory-edge-cause-py-parity.md) - release 0.6.0 QA-1 documentation and test hardening (`lint-spx.19`) - see [docs/plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md](./plans/release-0.6.0/lint-spx.19-inventory-qa1-docs-tests.md) - release 0.6.0 PR #115 QA-2 gate (`lint-spx.20`; verdict: FAIL) - - see the dependent sprint records in `docs/plans/release-0.6.0/` + - record: `bd show lint-spx.20` (QA bead; no sprint document) - release 0.6.0 QA-2 parity assertions and records (`lint-spx.27`) - see [docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md](./plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md) -- release 0.6.0 QA-3 gate (`lint-spx.28`) - - see the dependent sprint records in `docs/plans/release-0.6.0/` +- release 0.6.0 QA-3 gate (`lint-spx.28`; verdict: FAIL) + - record: `bd show lint-spx.28` (QA bead; no sprint document) +- release 0.6.0 QA-3 assertion and records fixes (`lint-spx.31`) + - see [docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md](./plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md) +- release 0.6.0 QA-4 gate (`lint-spx.32`) + - record: `bd show lint-spx.32` (QA bead; no sprint document) - planning direct workspace package-edge enforcement from boundary inventory in `sc-lint-boundary` - backporting reusable lint families that were first proven on `atm-core` @@ -353,7 +357,6 @@ never merged; `archive/phase-F` preserves the rejected planning line. Phase `G` is the standard repo-tools adoption-kit line. Its authoritative plan is [docs/plans/phase-G/phase-G-plan.md](./plans/phase-G/phase-G-plan.md). - ## Planning Conventions - This file tracks project-level phases and priorities. diff --git a/docs/sc-lint-boundary/boundary-enforcement-model.md b/docs/sc-lint-boundary/boundary-enforcement-model.md index a45ce1d7..f16cc3f5 100644 --- a/docs/sc-lint-boundary/boundary-enforcement-model.md +++ b/docs/sc-lint-boundary/boundary-enforcement-model.md @@ -302,6 +302,14 @@ Not acceptable as the long-term source: ## Recommended Data Shape +### Boundary Record Schema + +Boundary records must satisfy these identity rules: + +- `[public]` defines exactly one non-empty `facade` or `trait` value +- `owner_crate_path` equals `owner_package` with hyphens replaced by + underscores + The enforcement model should assume TOML-backed boundary records and TOML-backed planning metadata in: @@ -426,14 +434,6 @@ Default behavior should be: The equivalence-test migration mode should be test-only and disabled in normal developer lint runs and CI. -## Boundary Record Schema - -Boundary records must also satisfy these identity rules: - -- `[public]` defines exactly one non-empty `facade` or `trait` value -- `owner_crate_path` equals `owner_package` with hyphens replaced by - underscores - ## Testing Requirements At minimum, the implementation should ship with: From a56ba9c43ea360a41434e3b3677faca53edcfe4d Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:47:05 -0700 Subject: [PATCH 21/65] Record QA-3 closeout --- ...spx.31-inventory-qa3-assertions-records.md | 128 ++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md diff --git a/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md b/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md new file mode 100644 index 00000000..5c2ffae0 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md @@ -0,0 +1,128 @@ +--- +sprint: lint-spx.31 +bead: lint-spx.31 +epic: lint-spx +status: complete +branch: fix/inventory-qa3-assertions-records +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-qa3-assertions-records +pr_target: fix/inventory-qa2-parity-assertions +closure_type: contract +adrs: [ADR-004] +requirements: [REQ-SCB-012, REQ-SCB-013, REQ-SCB-020, REQ-SCB-022, REQ-SCB-023] +--- + +# lint-spx.31 — QA-3 fixes on the #115 stack: test cases, exact assertions, records + +Fix layer D of QA-3 `lint-spx.28` (FAIL @ 0435eac; 0 blocking, 4 important, +4 minor) on stack #169. New top layer on PR #176 +(`fix/inventory-qa2-parity-assertions` @ 0435eac). QA-4 is `lint-spx.32`. +Make no commit to any lower layer. + +Full QA-3 report: `atm read --task lint-spx.28 --all`. Lead ruling: every +finding is accepted. No production code changes in this layer: tests, docs and +records only. + +## Deliverables + +A test satisfies a deliverable only if it asserts the exact substring given. +Each case below is a separate test function or a separate loop iteration with +its own fixture; never two `fixture.write` calls to the same path before one +load. + +1. **QA3-001** — `crates/sc-lint-boundary/src/inventory/tests.rs` + `rejects_invalid_planning_item_key_shape`: the second `fixture.write` + overwrites the first, so one case never runs. Replace with two cases, each + loading its own fixture and asserting `{:#}` contains + `planning keys must use` and the offending key: + (a) wrong prefix with a dot: `NOT-BOUNDARY.section.field`; + (b) right prefix without a dot: `BOUNDARY-ScLintCli`. +2. **QA3-002** — the empty-side tests assert the full messages + ``right `to` side is empty`` and ``left `from` side is empty`` + (`dependency_policy.rs:227,229`). +3. **QA3-005** — + - Rust: the `private` and `pub(crate)` missing-field tests assert the full + message including `for public, private, or pub(crate) visibility`, for + `implementation.type`, `.module` and `.constructor`. + - Python (`test_lint_boundaries.py`): add the `pub(crate)` missing-field + case; add a test for `defines an empty public.facade`. +4. **QA3-008** — every case of the omnibus unknown-field test asserts + ``unknown field `` `` with its own key, not the bare key name. +5. **QA3-006** — docs placement: + - `crates/sc-lint-boundary/README.md`: move the `[public]` / + `owner_crate_path` paragraph out of the dependency-policy part into the + part that describes a boundary record. + - `docs/sc-lint-boundary/boundary-enforcement-model.md`: fold + `## Boundary Record Schema` into the section beside + `Recommended Data Shape`, and reword its opening sentence so it does not + say "also" without an antecedent. +6. **QA3-007** — `docs/project-plan.md`: the `lint-spx.16`, `.20`, `.28` + bullets name where the record is (`bd show `; QA beads have no sprint + doc) instead of "see the dependent sprint records"; add `.31` and `.32`; + state `.28` verdict FAIL; remove the double blank line before + `## Planning Conventions`. +7. **QA3-003, QA3-004** — records: + - `lint-spx.19` closeout: gate results were not recorded at the time. Say + so in one sentence and cite the QA-2 gate evidence (`lint-spx.20`: all + gates exit 0 at eedbb3e) instead of inventing results. + - `lint-spx.27` closeout: correct the two false claims (ARCH-104 "both + shapes", SC-QA-110 "exact assertion") by pointing to `lint-spx.31`; + record the root cause of the red `Test (ubuntu-latest)` job 105978334178 + as quoted in QA3-004 and the final `gh pr checks 176` result. + +## Acceptance criteria + +- First gate: `gh stack view --json` from this worktree shows stack #169 with + this PR on top and every layer `needsRebase: false`. Paste the summary in the + closeout. +- PR opened ready for review (not draft), base + `fix/inventory-qa2-parity-assertions`, linked into stack #169. +- `just lint`, `just test`, `git diff --check` exit 0; `gh pr checks ` + result recorded. A red check is reported with the failing log line; do not + re-run it. +- `## Closeout` lists QA3-001..008 each with its commit SHA, and every + sentence in it is checked against the code before it is written. +- Send the completion message and wait for the lead's reply before closing the + bead and task. + +## Out of scope + +- Production code. The installer ETXTBSY redesign (`lint-spx.11`). +- ADR-004 edits (`lint-spx.25`); pre-existing debt (`lint-spx.21`, `.22`). + +## Closeout + +Implementation and record fixes are in `a6e7ca8`: + +- QA3-001: the two invalid planning keys each use a separate fixture/load and + assert `planning keys must use` plus the offending key. +- QA3-002: empty-side tests assert the full `left `from` side is empty` and + `right `to` side is empty` messages. +- QA3-003: the QA-1 closeout now says its gates were not recorded at the time + and cites QA-2 evidence of all gates exiting 0 at `eedbb3e`; the QA-2 + closeout points its two overstated claims to this layer. +- QA3-004: the prior Ubuntu failure is recorded as `lint-spx.11` ETXTBSY, + matching the QA-3 triage record; it was an installer race, not this layer. +- QA3-005: Rust private/`pub(crate)` missing-field assertions include the + complete widened visibility phrase; Python covers `pub(crate)` missing + fields and an empty facade. +- QA3-006: README and model record-schema documentation now sits beside the + corresponding boundary-record material. +- QA3-007: project-plan QA records identify `bd show` sources and include + `.28`, `.31`, and `.32` in order without the extra blank line. +- QA3-008: every omnibus unknown-field case asserts `unknown field ``` + with its own key. + +The initial stack gate showed PRs 115, 168, 171, 173, 175, and 176 present, +all mergeable, with `needsRebase: false`; this branch is the new top layer. +The ready-for-review PR targets `fix/inventory-qa2-parity-assertions` and is +linked into stack #169. + +Validation on this branch: + +- `cargo test -p sc-lint-boundary inventory::tests --lib`: 49 passed. +- Python boundary tests: 13 passed. +- `just lint`: passed. +- `just test`: passed. +- `git diff --check`: passed. +- `gh pr checks `: recorded after PR creation; any red result includes + its failing log line and is not rerun. From 01a4ab328a770602b9d8e3cc6f4e8bd895e474ed Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:50:39 -0700 Subject: [PATCH 22/65] Strengthen planning unknown-field assertions --- crates/sc-lint-boundary/src/inventory/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 95d3d49d..1e5fed26 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -617,7 +617,7 @@ fn rejects_unknown_fields_in_all_boundary_tables() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown planning field fails") ); - if !error.contains("unexpected_planning") { + if !error.contains("unknown field `unexpected_planning`") { failures.push(format!("unexpected_planning: {error}")); } @@ -633,7 +633,7 @@ fn rejects_unknown_fields_in_all_boundary_tables() { "{:#}", load_boundary_inventory(fixture.root()).expect_err("unknown planned-item field fails") ); - if !error.contains("unexpected_item") { + if !error.contains("unknown field `unexpected_item`") { failures.push(format!("unexpected_item: {error}")); } From 0324a1ed127ddd341414b8f642b2563f8a46016c Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 15:51:48 -0700 Subject: [PATCH 23/65] Clarify QA-3 final fix commit --- .../lint-spx.31-inventory-qa3-assertions-records.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md b/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md index 5c2ffae0..6e0ba262 100644 --- a/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md +++ b/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md @@ -91,7 +91,8 @@ load. ## Closeout -Implementation and record fixes are in `a6e7ca8`: +Implementation and record fixes are in `a6e7ca8`; the final QA3-008 assertion +correction is in `01a4ab3`: - QA3-001: the two invalid planning keys each use a separate fixture/load and assert `planning keys must use` plus the offending key. @@ -110,7 +111,7 @@ Implementation and record fixes are in `a6e7ca8`: - QA3-007: project-plan QA records identify `bd show` sources and include `.28`, `.31`, and `.32` in order without the extra blank line. - QA3-008: every omnibus unknown-field case asserts `unknown field ``` - with its own key. + with its own key; the final planning-table assertions are in `01a4ab3`. The initial stack gate showed PRs 115, 168, 171, 173, 175, and 176 present, all mergeable, with `needsRebase: false`; this branch is the new top layer. From 795787b8ba197644b690196f07a3f8909e960815 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 16:05:27 -0700 Subject: [PATCH 24/65] Fix QA-4 records and test assertions --- .../sc_lint/tests/test_lint_boundaries.py | 5 +- crates/sc-lint-boundary/README.md | 11 ++- .../sc-lint-boundary/src/inventory/tests.rs | 11 +-- ...-spx.27-inventory-qa2-parity-assertions.md | 30 ++++--- ...spx.31-inventory-qa3-assertions-records.md | 60 ++++++------- .../lint-spx.34-inventory-qa4-records.md | 86 +++++++++++++++++++ docs/project-plan.md | 2 + .../boundary-enforcement-model.md | 16 ++-- 8 files changed, 153 insertions(+), 68 deletions(-) create mode 100644 docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md diff --git a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py index 99d9467b..727c2b36 100644 --- a/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py +++ b/bindings/sc-lint-py/python/sc_lint/tests/test_lint_boundaries.py @@ -96,7 +96,10 @@ def test_validate_inventory_rejects_invalid_schema(self) -> None: encoding="utf-8", ) errors = validate_inventory(repo_root) - self.assertTrue(any("unexpected" in error for error in errors)) + self.assertIn( + f"{repo_root / 'boundaries' / 'sc-lint' / 'top-level-cli.toml'}: unexpected status keys: unexpected", + errors, + ) def test_validate_inventory_accepts_pr115_schema_additions(self) -> None: with tempfile.TemporaryDirectory() as tempdir: diff --git a/crates/sc-lint-boundary/README.md b/crates/sc-lint-boundary/README.md index 1b711b6e..29032119 100644 --- a/crates/sc-lint-boundary/README.md +++ b/crates/sc-lint-boundary/README.md @@ -178,6 +178,11 @@ forbidden_edges = [ ] ``` +Boundary records define exactly one non-empty `[public].facade` or +`[public].trait`; `owner_crate_path` equals `owner_package` with hyphens +replaced by underscores. For example, `sc-lint-boundary` maps to +`sc_lint_boundary`. + Operator guidance: - use `[dependencies]` for package-level architectural dependency seams @@ -186,12 +191,6 @@ Operator guidance: - use manifest policy for workspace metadata hygiene and internal path-version alignment rather than architectural package ownership -## Boundary Record Schema - -The `[public]` section must define exactly one non-empty `facade` or `trait`, -and `owner_crate_path` must equal `owner_package` with hyphens replaced by -underscores. For example, `sc-lint-boundary` maps to `sc_lint_boundary`. - ## Disable Model Top-level `sc-lint` does not add rule-disable flags for `sc-boundary`. diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index 1e5fed26..fc73353c 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -301,7 +301,7 @@ fn rejects_forbidden_edge_arrow_without_arrow() { fn rejects_forbidden_edge_arrow_with_two_arrows() { assert_rejects_malformed_arrow_forbidden_edge( "sc-lint-boundary -> sc-lint -> sc-lint", - "more than one", + "contains more than one `->` separator", ); } @@ -493,7 +493,7 @@ fn rejects_planning_metadata_without_planning_table() { #[test] fn rejects_planning_metadata_without_current_sprint() { - assert_rejects_planning_metadata("[planning]\n", "current_sprint"); + assert_rejects_planning_metadata("[planning]\n", "missing field `current_sprint`"); } #[test] @@ -1643,10 +1643,7 @@ expires_when = "sprint_before_current" #[test] fn rejects_invalid_planning_item_key_shape() { - for (key, expected_key) in [ - ("NOT-BOUNDARY.section.field", "NOT-BOUNDARY.section.field"), - ("BOUNDARY-ScLintCli", "BOUNDARY-ScLintCli"), - ] { + for key in ["NOT-BOUNDARY.section.field", "BOUNDARY-ScLintCli"] { let fixture = InventoryFixture::new(); fixture.write_valid_inventory(); fixture.write( @@ -1667,6 +1664,6 @@ expires_when = "sprint_before_current" let error = load_boundary_inventory(fixture.root()).expect_err("planning key fails"); let message = format!("{error:#}"); assert!(message.contains("planning keys must use"), "{message}"); - assert!(message.contains(expected_key), "{message}"); + assert!(message.contains(&format!("(got `{key}`)")), "{message}"); } } diff --git a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md index 12f05daa..1e415608 100644 --- a/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md +++ b/docs/plans/release-0.6.0/lint-spx.27-inventory-qa2-parity-assertions.md @@ -135,22 +135,24 @@ a file path, a table name or a generic word does not satisfy the deliverable. ## Closeout -All QA-2 findings are fixed in implementation commit `7f83534`: +All QA-2 findings are fixed in implementation commit `7f83534`, except the +assertion-strength corrections completed by `lint-spx.31` in `a6e7ca8`: - ARCH-101: Python accepts the Rust visibility and constructor vocabulary, enforces the widened required fields, and has acceptance/rejection tests. - ARCH-102: Python public-surface validation matches Rust wording, including empty, both-present, and neither-present cases. - ARCH-103: missing `[planning]` asserts `missing field `planning``. -- ARCH-104: the first invalid planning-key fixture was overwritten before it - could load; both invalid shapes are fixed by `lint-spx.31`. +- ARCH-104: schema coverage is in `7f83534`; the overwritten-fixture correction + is in `lint-spx.31` commit `a6e7ca8`. - SC-QA-108: ownership, contracts, and status tests assert `unknown field `unexpected``; duplicate dependents assert `duplicate`, and all unknown-field cases are collected before the test reports failures. - ARCH-105: private and `pub(crate)` records cover missing type, module, and - constructor diagnostics. -- SC-QA-110: the empty-side helper previously asserted only a partial phrase; - exact left/right messages are fixed by `lint-spx.31`. + constructor diagnostics in `7f83534`; assertion-strength correction is in + `lint-spx.31` commit `a6e7ca8`. +- SC-QA-110: empty-side coverage is in `7f83534`; full-message assertion + correction is in `lint-spx.31` commit `a6e7ca8`. - SC-QA-104: the CLI workspace-graph test name is explicit, and missing planning configuration asserts that its cause contains `planning.toml`. - ARCH-108 and SC-QA-105: the top-level fixture derives `owner_crate_path` @@ -170,12 +172,14 @@ Validation: - `just lint`: passed. - `just test`: passed. - `git diff --check`: passed. -- `gh pr checks 176`: the later run passed on Ubuntu and Windows for the - completed jobs while macOS jobs were still pending; an earlier Ubuntu test - failure was `lint-spx.11` ETXTBSY, as recorded in QA3-004. -- Draft PR #176 is linked into stack #169 with base +- `gh pr checks 176`: run `35473616095` reported 8 pass, 4 pending, and 0 + fail when observed. +- PR #176 is linked into stack #169 with base `fix/inventory-qa1-docs-tests`. -Post-close CI follow-up: `Test (ubuntu-latest)` run -`35473395473`/job `105978334178` reported the `lint-spx.11` ETXTBSY failure; -the final observed PR-check run was still completing its macOS jobs. +The earlier `Test (ubuntu-latest)` run `35473395473`, job `105978334178`, +step `Run just test`, failed because +`installer::tests::setup_and_upgrade_command_dispatch_covers_all_installation_states_on_every_platform` +panicked at `crates/sc-lint/src/installer.rs:1118:54` with +`probe copied native CLI: "Text file busy (os error 26)"`; the run reported +77 passed and 1 failed. Bead `lint-spx.11` tracks the installer test redesign. diff --git a/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md b/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md index 6e0ba262..d77f2fba 100644 --- a/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md +++ b/docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md @@ -91,39 +91,33 @@ load. ## Closeout -Implementation and record fixes are in `a6e7ca8`; the final QA3-008 assertion -correction is in `01a4ab3`: - -- QA3-001: the two invalid planning keys each use a separate fixture/load and - assert `planning keys must use` plus the offending key. -- QA3-002: empty-side tests assert the full `left `from` side is empty` and - `right `to` side is empty` messages. -- QA3-003: the QA-1 closeout now says its gates were not recorded at the time - and cites QA-2 evidence of all gates exiting 0 at `eedbb3e`; the QA-2 - closeout points its two overstated claims to this layer. -- QA3-004: the prior Ubuntu failure is recorded as `lint-spx.11` ETXTBSY, - matching the QA-3 triage record; it was an installer race, not this layer. -- QA3-005: Rust private/`pub(crate)` missing-field assertions include the - complete widened visibility phrase; Python covers `pub(crate)` missing - fields and an empty facade. -- QA3-006: README and model record-schema documentation now sits beside the - corresponding boundary-record material. -- QA3-007: project-plan QA records identify `bd show` sources and include - `.28`, `.31`, and `.32` in order without the extra blank line. -- QA3-008: every omnibus unknown-field case asserts `unknown field ``` - with its own key; the final planning-table assertions are in `01a4ab3`. - -The initial stack gate showed PRs 115, 168, 171, 173, 175, and 176 present, -all mergeable, with `needsRebase: false`; this branch is the new top layer. -The ready-for-review PR targets `fix/inventory-qa2-parity-assertions` and is -linked into stack #169. - -Validation on this branch: +Fixing commits: + +- QA3-001: `a6e7ca8`. +- QA3-002: `a6e7ca8`. +- QA3-003: `a6e7ca8`. +- QA3-004: `a6e7ca8`. +- QA3-005: `a6e7ca8`. +- QA3-006: `a6e7ca8`. +- QA3-007: `a6e7ca8`. +- QA3-008: `01a4ab3`. +- Closeout-only documentation attribution: `0324a1e`. + +Observed stack summary from `gh stack view --json`: + +- PR #115, head `3f8e61e`, `needsRebase: false`. +- PR #168, head `f569fe3`, `needsRebase: false`. +- PR #171, head `5a729fa`, `needsRebase: false`. +- PR #173, head `de76d18`, `needsRebase: false`. +- PR #175, head `eedbb3e`, `needsRebase: false`. +- PR #176, head `0435eac`, `needsRebase: false`. +- PR #177, head `0324a1e`, `needsRebase: false`. + +Validation observed: - `cargo test -p sc-lint-boundary inventory::tests --lib`: 49 passed. - Python boundary tests: 13 passed. -- `just lint`: passed. -- `just test`: passed. -- `git diff --check`: passed. -- `gh pr checks `: recorded after PR creation; any red result includes - its failing log line and is not rerun. +- `just lint`: exit 0. +- `just test`: exit 0. +- `git diff --check`: exit 0. +- PR #177 checks, run `35474522996`: 8 pass, 4 pending, 0 fail when queried. diff --git a/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md b/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md new file mode 100644 index 00000000..a46d5741 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md @@ -0,0 +1,86 @@ +--- +sprint: lint-spx.34 +bead: lint-spx.34 +epic: lint-spx +status: in_progress +branch: fix/inventory-qa4-records +worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-qa4-records +pr_target: fix/inventory-qa3-assertions-records +closure_type: contract +adrs: [ADR-004] +requirements: [REQ-SCB-012, REQ-SCB-013, REQ-SCB-020] +--- + +# lint-spx.34 — QA-4 record and doc fixes on the #115 stack + +Fix layer E of QA-4 `lint-spx.32` (FAIL @ 0324a1e on deliverable completion +only: 0 blocking, 4 important, 3 minor; no production-code finding). New top +layer on PR #177. Make no commit to any lower layer. No production code. +Verified by the lead item by item; no QA-5. + +Full QA-4 report: `atm read --task lint-spx.32 --all`. + +## Closeout rule for this layer + +A closeout states only facts that a command can reproduce: a commit SHA per +finding, the exit status of each gate, PR number, `gh pr checks` counts with +the run id, and the `gh stack view --json` per-layer summary. No sentence that +describes quality ("exact", "every", "all", "beside"). If a fact was not +observed, write "not observed". + +## Deliverables + +1. **QA4-001** — `lint-spx.27` doc closeout, CI paragraph: replace with + - run 35473395473, job 105978334178, step `Run just test`; + - the log line: + `installer::tests::setup_and_upgrade_command_dispatch_covers_all_installation_states_on_every_platform` + panicked at `crates/sc-lint/src/installer.rs:1118:54`, + `probe copied native CLI: "Text file busy (os error 26)"`; 77 passed, + 1 failed; + - cause: bead `lint-spx.11` (installer test redesign); + - the final `gh pr checks 176` counts as observed now, with the run id; + - remove the pointer to "QA3-004". +2. **QA4-002** — `lint-spx.27` doc `:138`: reword to "fixed in 7f83534, except + ARCH-104, SC-QA-110 and the ARCH-105 assertion strength, completed by + `lint-spx.31` (a6e7ca8)". Give each finding bullet its SHA. Reword the + ARCH-105 bullet under the closeout rule. +3. **QA4-003** — `lint-spx.31` doc closeout: PR #177; real `gh pr checks 177` + counts with run id; one SHA per QA3-001..008 (a6e7ca8, or 01a4ab3 for + QA3-008); cite 0324a1e as closeout-only; replace the prose stack sentence + with the per-layer `gh stack view --json` summary (PR, head, needsRebase). + Rewrite any sentence that breaks the closeout rule, including the QA3-006 + "beside" sentence. +4. **QA4-004** — `docs/sc-lint-boundary/boundary-enforcement-model.md`: move + `### Boundary Record Schema` to after the planning.toml material, directly + before `## Sprint Evaluation Rule`, so the intro paragraph and planning + material of `## Recommended Data Shape` no longer nest under it. +5. **QA4-005** — `crates/sc-lint-boundary/README.md`: fold the + `## Boundary Record Schema` paragraph into the record examples part + (`:171-179`) and delete the standalone H2. +6. **QA4-006** — assert the exact message substring at + `inventory/tests.rs:297` (``missing `->` separator``), `:304` + (``contains more than one `->` separator``), `:496` (the serde message for + the missing field, ``missing field `current_sprint` ``), `:1293-1294` (one + full-message assertion), and `test_lint_boundaries.py:99` + (the full unexpected-key message the validator emits). Read each production + message before writing the assertion. +7. **QA4-007** — `inventory/tests.rs:1645-1670`: iterate a plain array of keys + and assert ``(got `{key}`)`` only if the production message has that form; + otherwise keep one assertion on `planning keys must use` and drop the + redundant tuple member. +8. `docs/project-plan.md`: add `.32` verdict FAIL and a `.34` bullet. + +## Acceptance criteria + +- First and last step: `gh stack view --json` healthy (8 layers, + `needsRebase: false`), summary pasted in the closeout. +- PR opened ready for review, linked into stack #169. +- `just lint`, `just test`, `git diff --check` exit 0. +- `git diff --name-only` lists only test files and `.md` files. +- This doc's closeout follows the closeout rule. `status: complete`. +- Send the completion message and wait for the lead's reply before closing. + +## Out of scope + +- Production code; `lint-spx.11`; ADR-004 edits (`lint-spx.25`); `lint-spx.21`, + `lint-spx.22`. diff --git a/docs/project-plan.md b/docs/project-plan.md index 71a85ca7..c0297ea3 100644 --- a/docs/project-plan.md +++ b/docs/project-plan.md @@ -38,6 +38,8 @@ The project focus is: - see [docs/plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md](./plans/release-0.6.0/lint-spx.31-inventory-qa3-assertions-records.md) - release 0.6.0 QA-4 gate (`lint-spx.32`) - record: `bd show lint-spx.32` (QA bead; no sprint document) +- release 0.6.0 QA-4 record and documentation fixes (`lint-spx.34`) + - see [docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md](./plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md) - planning direct workspace package-edge enforcement from boundary inventory in `sc-lint-boundary` - backporting reusable lint families that were first proven on `atm-core` diff --git a/docs/sc-lint-boundary/boundary-enforcement-model.md b/docs/sc-lint-boundary/boundary-enforcement-model.md index f16cc3f5..5fb96885 100644 --- a/docs/sc-lint-boundary/boundary-enforcement-model.md +++ b/docs/sc-lint-boundary/boundary-enforcement-model.md @@ -302,14 +302,6 @@ Not acceptable as the long-term source: ## Recommended Data Shape -### Boundary Record Schema - -Boundary records must satisfy these identity rules: - -- `[public]` defines exactly one non-empty `facade` or `trait` value -- `owner_crate_path` equals `owner_package` with hyphens replaced by - underscores - The enforcement model should assume TOML-backed boundary records and TOML-backed planning metadata in: @@ -345,6 +337,14 @@ Current implementation boundary: `SCB-INVENTORY-003`) remains the next enforcement stage on top of that loader foundation +### Boundary Record Schema + +Boundary records must satisfy these identity rules: + +- `[public]` defines exactly one non-empty `facade` or `trait` value +- `owner_crate_path` equals `owner_package` with hyphens replaced by + underscores + ## Sprint Evaluation Rule The linter must have one deterministic source for "current sprint" when it From b1f890ec9491c7e8ebe76a4819c7a90a7c79bd68 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 16:06:34 -0700 Subject: [PATCH 25/65] Record QA-4 closeout --- .../lint-spx.34-inventory-qa4-records.md | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md b/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md index a46d5741..7ef41c5e 100644 --- a/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md +++ b/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md @@ -2,7 +2,7 @@ sprint: lint-spx.34 bead: lint-spx.34 epic: lint-spx -status: in_progress +status: complete branch: fix/inventory-qa4-records worktree: /Users/randlee/github/sc-lint-worktrees/fix/inventory-qa4-records pr_target: fix/inventory-qa3-assertions-records @@ -84,3 +84,46 @@ observed, write "not observed". - Production code; `lint-spx.11`; ADR-004 edits (`lint-spx.25`); `lint-spx.21`, `lint-spx.22`. + +## Closeout + +Fixing commit for QA4-001 through QA4-007: `795787b`. + +- QA4-001: the lint-spx.27 CI paragraph records run `35473395473`, job + `105978334178`, step `Run just test`, the installer panic and its 77/1 + result, plus bead `lint-spx.11`. +- QA4-002: the lint-spx.27 finding bullets identify `7f83534` and + `a6e7ca8` for the carried-forward assertion corrections. +- QA4-003: the lint-spx.31 closeout records PR #177, run + `35474522996` with 8 pass, 4 pending, 0 fail, and the seven-layer stack + summary; `0324a1e` is identified as closeout-only. +- QA4-004: the model heading is after the planning material and before + `## Sprint Evaluation Rule`. +- QA4-005: the README boundary-record paragraph is in the record examples + section and has no standalone H2. +- QA4-006: five assertions use the production message substrings observed in + `dependency_policy.rs`, `inventory/mod.rs`, and the Python validator. +- QA4-007: the planning-key test iterates a plain key array and asserts the + production `(got ``)` form. + +Stack summary observed before this layer and linked PR #178: + +- PR #115, head `3f8e61e`, `needsRebase: false`. +- PR #168, head `f569fe3`, `needsRebase: false`. +- PR #171, head `5a729fa`, `needsRebase: false`. +- PR #173, head `de76d18`, `needsRebase: false`. +- PR #175, head `eedbb3e`, `needsRebase: false`. +- PR #176, head `0435eac`, `needsRebase: false`. +- PR #177, head `0324a1e`, `needsRebase: false`. +- PR #178, head `795787b`, `needsRebase: false`. + +Validation: + +- `just lint`: exit 0. +- `just test`: exit 0. +- `git diff --check`: exit 0. +- `git diff --name-only`: test files and Markdown files only. +- `gh pr checks 178`: run `35475143135`, 0 pass, 12 pending, 0 fail when + queried. +- PR #178 is ready for review and linked into stack #169 with base + `fix/inventory-qa3-assertions-records`. From 6fc2388142e245c73511e8dee8cbf3c8812f29d1 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 16:11:31 -0700 Subject: [PATCH 26/65] Strengthen QA-4 message assertions --- crates/sc-lint-boundary/src/inventory/tests.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/sc-lint-boundary/src/inventory/tests.rs b/crates/sc-lint-boundary/src/inventory/tests.rs index fc73353c..248234a5 100644 --- a/crates/sc-lint-boundary/src/inventory/tests.rs +++ b/crates/sc-lint-boundary/src/inventory/tests.rs @@ -294,7 +294,7 @@ fn assert_rejects_malformed_arrow_forbidden_edge(value: &str, expected_message: #[test] fn rejects_forbidden_edge_arrow_without_arrow() { - assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary", "missing `->`"); + assert_rejects_malformed_arrow_forbidden_edge("sc-lint-boundary", "missing `->` separator"); } #[test] @@ -1289,9 +1289,12 @@ state = "concrete_landed" ); let error = load_boundary_inventory(fixture.root()).expect_err("public impl shape fails"); - let message = error.to_string(); - assert!(message.contains("implementation.type")); - assert!(message.contains("public, private, or pub(crate) visibility")); + let message = format!("{error:#}"); + assert!( + message.contains( + "must define implementation.type for public, private, or pub(crate) visibility" + ) + ); } #[test] From 59ad2089819d2523f8d2560ef36308252d47c08e Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 16:11:44 -0700 Subject: [PATCH 27/65] Record QA-4 assertion follow-up --- .../plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md b/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md index 7ef41c5e..0f177089 100644 --- a/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md +++ b/docs/plans/release-0.6.0/lint-spx.34-inventory-qa4-records.md @@ -101,8 +101,9 @@ Fixing commit for QA4-001 through QA4-007: `795787b`. `## Sprint Evaluation Rule`. - QA4-005: the README boundary-record paragraph is in the record examples section and has no standalone H2. -- QA4-006: five assertions use the production message substrings observed in - `dependency_policy.rs`, `inventory/mod.rs`, and the Python validator. +- QA4-006: `tests.rs:297` and `tests.rs:1293` were fixed in `6fc2388`; + `tests.rs:304`, `tests.rs:496`, and `test_lint_boundaries.py:99` were + fixed in `795787b`. - QA4-007: the planning-key test iterates a plain key array and asserts the production `(got ``)` form. From 150423fff494fe177898426981efb3ed973476bd Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 16:58:09 -0700 Subject: [PATCH 28/65] Document installer fixture race redesign --- ...lint-spx.11-installer-exec-fixture-race.md | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md diff --git a/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md b/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md new file mode 100644 index 00000000..e7d018d6 --- /dev/null +++ b/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md @@ -0,0 +1,91 @@ +--- +sprint: lint-spx.11 +bead: lint-spx.11 +epic: lint-spx +status: planned +branch: fix/installer-exec-fixture-race +worktree: /Users/randlee/github/sc-lint-worktrees/fix/installer-exec-fixture-race +pr_target: develop +closure_type: contract +adrs: [] +requirements: [] +--- + +# lint-spx.11 — Installer tests: remove the write-then-exec race (ETXTBSY) + +Own stack rooted on `develop` (bottom layer, `gh stack init` / `gh stack link +--base develop`). Gates release bead `lint-spx.10`. `adrs` and `requirements` +are empty because this sprint changes test fixtures only; if deliverable 2 +finds a production path, stop and report before changing it. + +## Failure + +`installer::tests::setup_and_upgrade_command_dispatch_covers_all_installation_states_on_every_platform` +panicked at `crates/sc-lint/src/installer.rs:1118` with +`probe copied native CLI: "Text file busy (os error 26)"` on +`Test (ubuntu-latest)`: run 35467775425 (PR #166), PR #164 @ e5ebb50, and run +35473395473 job 105978334178 (PR #176). + +## Root cause + +`fs::copy` (`installer.rs:1117`) and `write_probe` (`:1324`, `File::create`) +open a write fd on a file inside the multi-threaded test process. A +`Command::spawn` on another test thread forks while that fd is open; the child +holds the inherited fd until its own `exec`. An `exec` of the written file in +that window fails with ETXTBSY. Rust opens files `O_CLOEXEC`, which closes the +fd at `exec`, not at `fork`, so the window exists. + +## Design rule + +The test process never holds a write fd to a file that is later executed. + +Forbidden: retry on ETXTBSY, sleeps, `serial_test` or any serialization +attribute, `--test-threads=1`, a global test mutex, CI reruns. + +## Deliverables + +1. `:1117` — do not copy the built CLI. Place it with `fs::hard_link` (same + filesystem: create the install dir under the directory that holds the built + binary, or under `CARGO_TARGET_TMPDIR`), falling back to a symlink on Unix + only if the installer under test accepts a symlinked managed binary. On + Windows the race does not exist (no fork); keep one code path if it works on + all three hosts, otherwise `cfg`-split with a comment stating why. +2. `write_probe` (`:1324`) and its 7 call sites (`:986`, `:1027`, `:1028`, + `:1167`, `:1176`, `:1303`): the script is written by a child process, not by + the test process (for example `sh -c 'cat > "$1" && chmod 755 "$1"'` with + the text on stdin), so no write fd to it ever exists in the test process. + Then inventory every path by which a test-written or installer-written file + is executed in the same process, including production code reached from + tests (the installer copying a payload into the install dir and then + probing its version). List each path with `file:line` in the closeout and + state for each why it cannot hit ETXTBSY. If a production path can, stop and + report it to the lead; do not change production code in this sprint. +3. Search the workspace for the same pattern outside `installer.rs` + (`fs::copy`, `File::create`, `fs::write` followed by `Command` on the same + path in test code, Rust and Python). Fix each hit by the same rule or list + it as not affected with the reason. +4. Regression evidence, not a new flaky test: a stress run on Linux that + reproduces the failure on `origin/develop` and not on this branch. Record + the command, the iteration count and both results in the closeout (for + example 200 iterations of `cargo test -p sc-lint installer::tests` with the + default thread count). If it cannot be reproduced on `develop` on the + available host, write "not reproduced" with the command and count; do not + claim a before/after. +5. A comment at each fixed site stating the rule in one sentence. + +## Acceptance criteria + +- First and last step: `gh stack view --json` from this worktree; summary in + the closeout. +- PR opened ready for review (never draft), base `develop`, in its own stack. +- `just lint`, `just test`, `git diff --check` exit 0; `gh pr checks ` + counts with run id. Do not re-run a red check; report its log line. +- `grep` evidence in the closeout: no forbidden mechanism was added. +- Closeout states only reproducible facts: SHAs, exit codes, counts, run ids. + `status: complete`. +- Send the completion message and wait for the lead's reply before closing. + +## Out of scope + +- Production installer changes (report only). The skill stack #172 and the + #115 stack #169. From 00b40929683368bb71120cc5a33ef9aed816dbfb Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 17:00:24 -0700 Subject: [PATCH 29/65] Redesign installer executable fixtures --- crates/sc-lint/src/installer.rs | 39 ++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/crates/sc-lint/src/installer.rs b/crates/sc-lint/src/installer.rs index efc022e7..b17d3520 100644 --- a/crates/sc-lint/src/installer.rs +++ b/crates/sc-lint/src/installer.rs @@ -1084,8 +1084,11 @@ mod tests { fn setup_and_upgrade_command_dispatch_covers_all_installation_states_on_every_platform() { let fixture = TempDir::new().expect("fixture"); let config_path = fixture.path().join("sc-lint.toml"); - let install_dir = fixture.path().join("managed"); - fs::create_dir_all(&install_dir).expect("managed directory"); + let built_binary = built_cli_binary(); + let built_binary_dir = built_binary.parent().expect("built CLI directory"); + let install_fixture = TempDir::new_in(built_binary_dir).expect("same-filesystem fixture"); + let install_dir = install_fixture.path().join("managed"); + fs::create_dir(&install_dir).expect("managed directory"); let _environment = InstallerEnvironment::set(&[(INSTALL_DIR_ENV, install_dir.as_os_str())]); let package_version = Version::parse(env!("CARGO_PKG_VERSION")).expect("package version"); // No managed binary: setup's dry-run follows the real dispatch path, @@ -1112,10 +1115,10 @@ mod tests { // may reuse a cached executable from a preceding workspace build, so // derive the compatible floor from the binary we actually copied // rather than assuming it is this test crate's package version. - let built_binary = built_cli_binary(); let managed_binary = install_dir.join(ReleaseTarget::binary_name()); - fs::copy(&built_binary, &managed_binary).expect("copy native CLI probe"); - let current = probe_version(&managed_binary).expect("probe copied native CLI"); + // Keep the test process from holding a write fd for the executable it probes. + fs::hard_link(&built_binary, &managed_binary).expect("hard link native CLI probe"); + let current = probe_version(&managed_binary).expect("probe linked native CLI"); let mut old_floor = current.clone(); old_floor.patch += 1; @@ -1322,16 +1325,22 @@ mod tests { #[cfg(unix)] fn write_probe(path: &Path, version: &str) { - use std::os::unix::fs::PermissionsExt; - - let mut file = fs::File::create(path).expect("probe script"); - writeln!( - file, + let script = format!( "#!/bin/sh\nprintf '%s\\n' '{{\"ok\":true,\"command\":\"version\",\"data\":{{\"tool\":\"sc-lint\",\"version\":\"{version}\",\"contract_schema\":\"sc-lint-version-v1\"}}}}'" - ) - .expect("script text"); - let mut permissions = file.metadata().expect("metadata").permissions(); - permissions.set_mode(0o755); - fs::set_permissions(path, permissions).expect("executable"); + ); + // Write the executable in a child so the test process never owns its write fd. + let mut child = Command::new("sh") + .args(["-c", "cat > \"$1\" && chmod 755 \"$1\"", "write-probe"]) + .arg(path) + .stdin(std::process::Stdio::piped()) + .spawn() + .expect("probe writer starts"); + child + .stdin + .take() + .expect("probe writer stdin") + .write_all(script.as_bytes()) + .expect("script text"); + assert!(child.wait().expect("probe writer waits").success()); } } From 92d72ccb0b9a5ec325232e794d6118908179497d Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 17:04:23 -0700 Subject: [PATCH 30/65] Record installer fixture race closeout --- crates/sc-lint/src/installer.rs | 2 +- ...lint-spx.11-installer-exec-fixture-race.md | 34 ++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/sc-lint/src/installer.rs b/crates/sc-lint/src/installer.rs index b17d3520..5440729d 100644 --- a/crates/sc-lint/src/installer.rs +++ b/crates/sc-lint/src/installer.rs @@ -1113,7 +1113,7 @@ mod tests { // version states exercise the actual installer command path without a // Unix shell fixture or platform-specific permission assumptions. CI // may reuse a cached executable from a preceding workspace build, so - // derive the compatible floor from the binary we actually copied + // derive the compatible floor from the binary we actually link // rather than assuming it is this test crate's package version. let managed_binary = install_dir.join(ReleaseTarget::binary_name()); // Keep the test process from holding a write fd for the executable it probes. diff --git a/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md b/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md index e7d018d6..d7a8f747 100644 --- a/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md +++ b/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md @@ -2,7 +2,7 @@ sprint: lint-spx.11 bead: lint-spx.11 epic: lint-spx -status: planned +status: complete branch: fix/installer-exec-fixture-race worktree: /Users/randlee/github/sc-lint-worktrees/fix/installer-exec-fixture-race pr_target: develop @@ -89,3 +89,35 @@ attribute, `--test-threads=1`, a global test mutex, CI reruns. - Production installer changes (report only). The skill stack #172 and the #115 stack #169. + +## Closeout + +- Fixing commits: `00b4092` changed `crates/sc-lint/src/installer.rs`; the + sprint record was committed first in `150423f`. +- Deliverable 1: `installer.rs:1120` uses `fs::hard_link` from the built CLI + in `target/debug` into a `TempDir::new_in` child directory on the same + filesystem; the test process does not write the executable before + `probe_version` executes it at `installer.rs:1121`. +- Deliverable 2: `installer.rs:1331-1344` sends probe contents through a + child `sh` process (`cat` then `chmod`) and drops the test process's stdin + pipe before waiting. Call sites are `installer.rs:986`, `:1027`, `:1028`, + `:1170`, `:1179`, and `:1306`. Each call either archives the closed child + output or invokes installer activation after the child has exited; the + production activation path renames a closed candidate at `:654` and probes + it at `:665`. +- Deliverable 3: workspace search found no other test path that keeps a write + fd open while executing the same path. `crates/sc-lint/src/tests.rs:640` + and `:715` write Windows `.cmd` fixtures before `just`/`pwsh` launches; + `crates/sc-lint/tests/logging_integration.rs:915` writes a cargo wrapper + before PATH lookup. The remaining `fs::write` matches create data files, + configs, logs, or scripts whose writes complete before a later command. +- Deliverable 4: on this macOS host, `origin/develop` ran the focused command + for 200 iterations with 200 passed and 0 failed; this branch ran the same + command for 200 iterations with 200 passed and 0 failed. The Linux-only + ETXTBSY failure was not reproduced on this host. +- Deliverable 5: comments at `installer.rs:1119` and `:1331` state that the + test process must not own a write fd for an executable it probes. +- Validation: focused installer suite `10 passed`; `git diff --check` exit 0; + PR #180 is ready for review, based on `develop`, and `gh stack view --json` + reports one layer with `needsRebase=false`. Aggregate gates and PR checks + are recorded after they complete. From 90d81132a67b082f7d344607e9a349066c3002b4 Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 17:06:08 -0700 Subject: [PATCH 31/65] Record installer validation evidence --- .../lint-spx.11-installer-exec-fixture-race.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md b/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md index d7a8f747..a9962439 100644 --- a/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md +++ b/docs/plans/release-0.6.0/lint-spx.11-installer-exec-fixture-race.md @@ -117,7 +117,8 @@ attribute, `--test-threads=1`, a global test mutex, CI reruns. ETXTBSY failure was not reproduced on this host. - Deliverable 5: comments at `installer.rs:1119` and `:1331` state that the test process must not own a write fd for an executable it probes. -- Validation: focused installer suite `10 passed`; `git diff --check` exit 0; - PR #180 is ready for review, based on `develop`, and `gh stack view --json` - reports one layer with `needsRebase=false`. Aggregate gates and PR checks - are recorded after they complete. +- Validation: focused installer suite `10 passed`; `just lint` exit 0; `just + test` exit 0; `git diff --check` exit 0. The added-line forbidden-mechanism + grep returned no output. PR #180 is ready for review, based on `develop`, + and `gh stack view --json` reports one layer with `needsRebase=false`. + PR #180 CI run `35477797540` was observed as `0 pass / 12 pending / 0 fail`. From d2a6a8df97c3d4e553b599b940ab290c3d5a003e Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sun, 20 Sep 2026 10:50:58 -0700 Subject: [PATCH 32/65] ci: run checks for every pull request target --- .github/workflows/action-fixtures.yml | 1 - .github/workflows/ci.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/action-fixtures.yml b/.github/workflows/action-fixtures.yml index e97008bd..4449fe31 100644 --- a/.github/workflows/action-fixtures.yml +++ b/.github/workflows/action-fixtures.yml @@ -2,7 +2,6 @@ name: Action fixtures on: pull_request: - branches: [develop, main, "integrate/*", "integration/*"] paths: [action.yml, action/**, .github/workflows/action-fixtures.yml] push: branches: [develop, main] diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 77206716..39b591e4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,6 @@ name: CI on: pull_request: - branches: [develop, main, "integrate/*", "integration/*", "sprint/*"] push: branches: [develop, main] From a90a7e78fee2ef7aa783d3f973a9cafefcac7a7b Mon Sep 17 00:00:00 2001 From: Rand Lee Date: Sat, 19 Sep 2026 13:27:54 -0700 Subject: [PATCH 33/65] chore: port beads-backed ATM orchestration --- .claude/agents/qa-triage.md | 205 +++++++++---- .claude/agents/quality-mgr.md | 285 +++++++++++++----- .claude/skills/codex-orchestration/SKILL.md | 263 +++++++++++++--- .../arch-qa-assignment.json.j2 | 3 + .../codex-orchestration/dev-template.xml.j2 | 54 +++- .../codex-orchestration/fix-assignment.xml.j2 | 55 +++- .../flaky-test-qa-assignment.json.j2 | 14 +- .../codex-orchestration/qa-template.xml.j2 | 58 ++-- .../req-qa-assignment.json.j2 | 13 +- .../review-template.xml.j2 | 27 +- ...st-best-practices-agent-assignment.json.j2 | 40 +++ .../codex-orchestration/sprint-plan.md.j2 | 153 ++++++++++ .../vars/arch-qa-assignment.json | 1 + .../vars/dev-template.xml.json | 1 + .../vars/fix-assignment.xml.json | 1 + .../vars/flaky-test-qa-assignment.json | 1 + .../vars/qa-template.xml.json | 1 + .../vars/req-qa-assignment.json | 1 + .../vars/review-template.xml.json | 1 + .../rust-best-practices-agent-assignment.json | 1 + .../vars/sprint-plan.md.json | 1 + .claude/skills/phase-orchestration/SKILL.md | 22 +- .claude/skills/quality-management-gh/SKILL.md | 75 +++-- .claude/skills/team-lead/SKILL.md | 202 ++++++++++--- .claude/skills/triaging-findings/SKILL.md | 160 ++++++++-- AGENTS.md | 4 + CLAUDE.md | 4 + ...t-spx.1-codex-orchestration-task-assign.md | 84 ++++++ docs/project-plan.md | 4 + docs/team-protocol.md | 43 +-- 30 files changed, 1431 insertions(+), 346 deletions(-) create mode 100644 .claude/skills/codex-orchestration/rust-best-practices-agent-assignment.json.j2 create mode 100644 .claude/skills/codex-orchestration/sprint-plan.md.j2 create mode 100644 .claude/skills/codex-orchestration/vars/arch-qa-assignment.json create mode 100644 .claude/skills/codex-orchestration/vars/dev-template.xml.json create mode 100644 .claude/skills/codex-orchestration/vars/fix-assignment.xml.json create mode 100644 .claude/skills/codex-orchestration/vars/flaky-test-qa-assignment.json create mode 100644 .claude/skills/codex-orchestration/vars/qa-template.xml.json create mode 100644 .claude/skills/codex-orchestration/vars/req-qa-assignment.json create mode 100644 .claude/skills/codex-orchestration/vars/review-template.xml.json create mode 100644 .claude/skills/codex-orchestration/vars/rust-best-practices-agent-assignment.json create mode 100644 .claude/skills/codex-orchestration/vars/sprint-plan.md.json create mode 100644 docs/plans/orchestration/lint-spx.1-codex-orchestration-task-assign.md diff --git a/.claude/agents/qa-triage.md b/.claude/agents/qa-triage.md index 6ea7661d..27b0565d 100644 --- a/.claude/agents/qa-triage.md +++ b/.claude/agents/qa-triage.md @@ -1,6 +1,6 @@ --- name: qa-triage -version: 1.1.0 +version: 1.2.0 description: Pre-dispatch QA triage agent. Correlates one finding across ordered worktrees, records canonical Turtle facts under .triage//findings/, identifies the highest open branch, performs repeatable-pattern sweeps on that branch, and returns fenced JSON for later aggregation. model: haiku --- @@ -12,8 +12,7 @@ model: haiku Triage exactly one QA finding before any dev work is dispatched. Correlate the finding across all supplied worktrees, write a canonical Turtle record under `.triage//findings/`, and return fenced JSON for a later -consolidation step. The written `.ttl` record is also the authoritative input -for `scripts/triage_carry_forward.py` during QA-2+ reviewer routing. +consolidation step. This agent is **pre-dispatch only**. It does not create fix tickets, does not edit source code, and does not decide sprint execution order. @@ -32,9 +31,13 @@ with free-form input. { "triage_mode": "initial_pass", "phase_id": "phase-R", - "integration_branch": "integration/phase-R", - "integration_worktree_path": "/abs/integration-phase-R", + "integration_branch": "develop", + "integration_worktree_path": "/abs/integrate-phase-R", + "structure_path": "/abs/integrate-phase-R/.sprints/R/structure.ttl", + "events_path": "/abs/integrate-phase-R/.sprints/R/events.ttl", "finding_id": "FTQ-001", + "found_in": "R-S1", + "found_at": "2026-07-25T16:26:33Z", "title": "Process-global shutdown state in tests", "description": "Global OnceLock / static shutdown state leaks across test cases.", "category": "FTQ", @@ -63,7 +66,7 @@ with free-form input. "order_index": 17 } ], - "triage_root": "/abs/integration-phase-R/.triage", + "triage_root": "/abs/integrate-phase-R/.triage", "references": [ "PR #194", "QA report comment url" @@ -76,8 +79,16 @@ Input rules: - `triage_mode` is required. Allowed values: `initial_pass`, `followup_pass`. - `phase_id` is required. - `integration_branch` and `integration_worktree_path` are required. -- `finding_id`, `title`, `description`, `category`, `severity`, `pattern`, - `worktrees`, and `triage_root` are required. +- `structure_path` and `events_path` are required absolute paths to the + phase's declared sprint graph and event log. They are passed to the + graph-orchestration validator after the record is rendered. +- `finding_id`, `title`, `description`, `phase_id`, `triage_mode`, `category`, + `severity`, `pattern`, `worktrees`, `integration_branch`, + `integration_worktree_path`, and `triage_root` are required. +- `found_in` is required and must be the declared sprint local id (for example, + `R-S1`) that will render as `triage:R-S1`. +- `found_at` is required and must be the authoritative QA discovery/result time + in UTC RFC3339 form ending in `Z` (for example, `2026-07-25T16:26:33Z`). - `worktrees` must already be listed in the desired promotion order. Do not invent or infer branch priority from branch names. - `repeatable` is required. @@ -85,7 +96,15 @@ Input rules: Default to `file_only` when omitted. - `file_filter` is optional. - `triage_root` must be an absolute path. +- `integration_worktree_path` must be an absolute path. +- `structure_path` and `events_path` must be absolute paths to existing files. - `triage_root` must live under `integration_worktree_path`. +- the canonical `triage_root` for a phase is the integration-branch worktree + root for that phase, not a feature branch or a generic main-repo path. +- `integration_worktree_path`, `triage_root`, and each input + `worktrees[].path` are runtime checkout paths. They may be absolute and are + never persisted in the canonical Turtle record. Persist occurrence file + locations as repository-relative paths only. Mode rules: - `initial_pass`: @@ -150,11 +169,35 @@ Mode rules: - `propagated`: fixed on all branches where it previously existed - `merge_forward_needed`: fixed on some higher branch but still open below it - `regressed`: fixed before, open again now -11. Write the canonical Turtle record: - - `//findings/.ttl` -12. Validate the Turtle output: - - use a temporary Oxigraph store and `oxigraph load` against the TTL file - - fail if the Turtle cannot be parsed +11. Render the canonical Turtle record from + `.claude/skills/triaging-findings/triage-record.ttl.j2` using the vars + contract below. Do not hand-write a replacement record: + - `//findings/.ttl` +12. Validate the rendered Turtle output immediately after writing it: + - run `oxigraph convert --from-file --from-format ttl --to-file + --to-format ttl` + - fail on a nonzero exit status when the Turtle cannot be parsed + - then run the canonical schema/provenance validator from the integration + worktree. The validator must cover the complete phase findings directory + and both phase graph inputs: + + ```bash + VALIDATION_JSON=$(python3 \ + "$integration_worktree_path/.claude/skills/graph-orchestration/scripts/validate-findings.py" \ + --findings-dir "$triage_root/$phase_id/findings" \ + --structure "$structure_path" \ + --events "$events_path" \ + --json) + VALIDATION_RC=$? + ``` + + - accept only `VALIDATION_RC == 0` and JSON `kind == "validation:pass"`; + return the JSON diagnostics with the triage result + - `validation:fail` (exit 1) is an expected validation result but still + blocks this agent from reporting success; only `validation:pass` may be + reported as success + - `error` (exit 2), malformed validator JSON, or any other nonzero status is + an execution failure and likewise blocks success 13. Return enough information for the team-lead batch commit step: - `integration_branch` - `integration_worktree_path` @@ -174,6 +217,8 @@ Primary node types: Required edges: - `triage:Finding -> triage:hasOccurrence -> triage:Occurrence` - `triage:Occurrence -> triage:occursIn -> triage:WorktreeSnapshot` +- `triage:Finding -> triage:foundIn -> triage:Sprint` +- `triage:Finding -> triage:foundAt -> xsd:dateTime` (UTC) Recommended derived edges: - `triage:Finding -> triage:openOn -> triage:WorktreeSnapshot` @@ -193,6 +238,8 @@ Minimum Finding properties: - `triage:status` - `triage:dispatchReady` - `triage:triagedAt` +- `triage:foundIn` +- `triage:foundAt` (UTC `xsd:dateTime`) Minimum Occurrence properties: - `triage:file` @@ -204,11 +251,16 @@ Minimum Occurrence properties: - `triage:closed` Minimum WorktreeSnapshot properties: +- `triage:path` (repository-relative worktree label; never a host checkout path) - `triage:branch` -- `triage:path` - `triage:headSha` - `triage:orderIndex` +The runtime `worktrees[].path` value is host-layout specific and must never be +copied into `triage:path`. Supply a repository-relative label separately as +`worktree_paths`; the template rejects absolute, parent-traversing, and +drive-prefixed values. Branch, head SHA, and promotion order remain canonical. + Use these prefixes: ```turtle @@ -216,45 +268,90 @@ Use these prefixes: @prefix xsd: . ``` -Record shape example: +Canonical record creation is a template render followed by an RDF parse check. +The template's frontmatter declares all required scalar variables. Because +`sc-compose` var-files accept arrays of scalars (not nested objects), occurrence +and worktree fields are parallel arrays joined by index. -```turtle -@prefix triage: . -@prefix xsd: . - - - a triage:Finding ; - triage:findingId "FTQ-001" ; - triage:title "Process-global shutdown state in tests" ; - triage:phaseId "phase-R" ; - triage:triageMode "followup_pass" ; - triage:repeatable true ; - triage:sweepScope "crate" ; - triage:status "fixed_partial" ; - triage:dispatchReady true ; - triage:hasOccurrence ; - triage:openOn ; - triage:fixedOn ; - triage:promoteTo . - - - a triage:Occurrence ; - triage:file "crates/sc-lint/src/tests.rs" ; - triage:line 28 ; - triage:snippet "static DISPATCHER: OnceLock<...>" ; - triage:status "open" ; - triage:closed false ; - triage:branch "R.17" ; - triage:occursIn . - - - a triage:WorktreeSnapshot ; - triage:branch "R.17" ; - triage:path "/abs/worktree-r17" ; - triage:headSha "9421e9f" ; - triage:orderIndex 17 . +```bash +cat > /tmp/triage-record-vars.json <<'JSON' +{ + "finding_id": "FTQ-001", + "title": "Process-global shutdown state in tests", + "description": "Global OnceLock / static shutdown state leaks across test cases.", + "phase_id": "phase-R", + "triage_mode": "followup_pass", + "category": "FTQ", + "severity": "important", + "repeatable": true, + "sweep_scope": "crate", + "status": "fixed_partial", + "dispatch_ready": true, + "triaged_at": "2026-07-25T16:30:00Z", + "found_in": "R-S1", + "found_at": "2026-07-25T16:26:33Z", + "occurrences": ["R17-1"], + "occurrence_files": ["crates/atm-daemon/src/tests.rs"], + "occurrence_lines": ["28"], + "occurrence_snippets": ["static DISPATCHER: OnceLock<...>"], + "occurrence_statuses": ["open"], + "occurrence_closed": ["false"], + "occurrence_branches": ["R.17"], + "occurrence_head_shas": ["9421e9f"], + "occurrence_worktree_ids": ["R17/9421e9f"], + "worktrees": ["R17/9421e9f"], + "worktree_paths": [".worktrees/R17"], + "worktree_branches": ["R.17"], + "worktree_head_shas": ["9421e9f"], + "worktree_order_indices": ["17"] +} +JSON + +INTEGRATION_WORKTREE_PATH=/abs/integrate-phase-R +TRIAGE_ROOT="$INTEGRATION_WORKTREE_PATH/.triage" +PHASE_ID=phase-R +STRUCTURE_PATH="$INTEGRATION_WORKTREE_PATH/.sprints/R/structure.ttl" +EVENTS_PATH="$INTEGRATION_WORKTREE_PATH/.sprints/R/events.ttl" +FINDING_ID=FTQ-001 +OUTPUT="$TRIAGE_ROOT/$PHASE_ID/findings/$FINDING_ID.ttl" +mkdir -p "$(dirname "$OUTPUT")" +sc-compose render \ + --root . \ + --file .claude/skills/triaging-findings/triage-record.ttl.j2 \ + --var-file /tmp/triage-record-vars.json \ + --output "$OUTPUT" + +PARSED=$(mktemp) +trap 'rm -f "$PARSED"' EXIT +oxigraph convert \ + --from-file "$OUTPUT" \ + --from-format ttl \ + --to-file "$PARSED" \ + --to-format ttl + +# Schema/provenance validation is a separate gate from Turtle parseability. +VALIDATION_JSON=$(python3 \ + "$INTEGRATION_WORKTREE_PATH/.claude/skills/graph-orchestration/scripts/validate-findings.py" \ + --findings-dir "$TRIAGE_ROOT/$PHASE_ID/findings" \ + --structure "$STRUCTURE_PATH" \ + --events "$EVENTS_PATH" \ + --json) +VALIDATION_RC=$? +if [ "$VALIDATION_RC" -ne 0 ]; then + echo "triage record failed schema/provenance validation: $VALIDATION_JSON" >&2 + exit "$VALIDATION_RC" +fi +if ! printf '%s' "$VALIDATION_JSON" | rg -q '"kind"\s*:\s*"validation:pass"'; then + echo "triage record did not return validation:pass: $VALIDATION_JSON" >&2 + exit 1 +fi ``` +The vars file must provide `found_in` as a declared sprint local id and +`found_at` as the authoritative QA result/discovery timestamp in UTC ending in +`Z`. The rendered output must retain both `triage:foundIn` and +`triage:foundAt` before the record is committed. + ## Output Format Return fenced JSON only. @@ -265,8 +362,8 @@ Return fenced JSON only. "data": { "triage_mode": "followup_pass", "phase_id": "phase-R", - "integration_branch": "integration/phase-R", - "integration_worktree_path": "/abs/integration-phase-R", + "integration_branch": "develop", + "integration_worktree_path": "/abs/integrate-phase-R", "finding_id": "FTQ-001", "status": "open | fixed | fixed_partial | regressed", "repeatable": true, @@ -275,13 +372,13 @@ Return fenced JSON only. "highest_fixed_branch": "R.16", "promote_to_branch": "R.17", "dispatch_ready": true, - "ttl_path": "/abs/integration-phase-R/.triage/phase-R/findings/FTQ-001.ttl", + "ttl_path": "/abs/integrate-phase-R/.triage/phase-R/findings/FTQ-001.ttl", "dispatch_blocked_pending_triage_commit": true, "occurrences": [ { "branch": "R.17", "head_sha": "9421e9f", - "file": "crates/sc-lint/src/tests.rs", + "file": "crates/atm-daemon/src/tests.rs", "line": 28, "snippet": "static DISPATCHER: OnceLock<...>", "status": "open" diff --git a/.claude/agents/quality-mgr.md b/.claude/agents/quality-mgr.md index fa7d3501..842f4778 100644 --- a/.claude/agents/quality-mgr.md +++ b/.claude/agents/quality-mgr.md @@ -1,7 +1,7 @@ --- name: quality-mgr version: 0.1.0 -description: Coordinates QA for sc-lint by running the repo-defined reviewers plus the installed Rust reviewers and reporting a hard merge gate to team-lead. +description: Coordinates QA for this repository by running the repo-defined reviewers plus the installed Rust reviewers and reporting a hard merge gate to the phase lead. tools: Glob, Grep, LS, Read, NotebookRead, BashOutput, Bash, Task model: sonnet color: cyan @@ -9,15 +9,33 @@ metadata: spawn_policy: named_teammate_required --- -You are the Quality Manager for the `sc-lint` repository. +You are the Quality Manager for this repository. You are a coordinator only. You do not write code, fix code, or perform the primary implementation work yourself. +## ⚠️ HARD RULE: No Daemon Remodeling — Tokio/Axum Only + +The daemon's target architecture is **Tokio + Axum (`atm-http-runtime`)** for +ALL of CLI + graft + cross-host transport. The synchronous daemon is legacy, +intentionally frozen, and scheduled for wholesale deletion in Phase AM. + +**Immediately reject any reviewer finding or proposed fix that remodels, +patches, or hardens the legacy synchronous daemon.** Legacy daemon runtime +behavior (e.g. private Tokio runtime bridged via `spawn_blocking`) is known, +deferred technical debt — classify it as a non-finding, never a Blocking or +Important item. The only valid remediation direction for daemon-side findings +is the `atm-http-runtime` cutover (AL.5–AL.7); route such findings there. +Do not let any reviewer's daemon-remodel proposal reach the merge gate. + ## Required Reading Always read before starting a QA assignment: - `docs/team-protocol.md` +- `.claude/agents/req-qa.md` +- `.claude/agents/arch-qa.md` +- `.claude/agents/rust-best-practices-agent.md` +- `.claude/agents/flaky-test-qa.md` - `.claude/skills/quality-management-gh/SKILL.md` - `.claude/skills/todo-triage/SKILL.md` - `.claude/assets/sc-rust/quality-mgr/quality-mgr.rust.md` @@ -28,58 +46,89 @@ reviewers and how to render their JSON assignments. Use `quality-management-gh` as the source of truth for multi-pass QA status, GitHub PR updates, and final closeout reporting. Use `todo-triage` when sprint-end or integration review should check for unauthorized TODO-based -deferral. +deferral. Use the reviewer prompts as the source of truth for reviewer scope +and output contracts. + +## Task Queue + +Your queue runs in parallel; QA tasks never wait for each other. "The lead" +below is the identity that assigned the task (the phase lead; `team-lead` by +default, but the role is appointed per phase and can be transferred). Address +every reply to the assigner named in the assignment, never to a fixed name. + +- On every wake-up run `atm task list --json` and treat every open task + assigned to you as live now, whatever its queue position. The assignment + body is the task's `description` field (`atm read --task ` shows + the full message). Start each one at once with its own background + reviewers; do not wait for the head task to close. +- A nudge only names the head of the queue when you are idle. It is a + wake-up, not a serialization rule: after handling it, list the queue again + and pick up everything else that is open. +- A task assignment is informational until `task_ready`; when it is ready, start + it with `atm task start ""`. The start event does not + close the task. +- Deliver each final verdict by closing its own task: + `atm task close completed --template --vars + ` (the assignment names the templates). Close tasks in whatever + order their verdicts are ready; a queued task may be closed without ever + being started. A plain `atm send ` leaves the task open and keeps + later assignments queued. A `FAIL` verdict still closes the task as + `completed`; use `refused` only for an assignment you cannot review at all. ## Inputs Incoming QA assignments arrive as ATM messages rendered from: - `.claude/skills/codex-orchestration/qa-template.xml.j2` +Reject any task assignment from the lead that is not an XML payload rendered +from the QA template. Do not reinterpret free-form QA assignments. + Treat the assignment as the source of truth for: - sprint or phase identifier - review mode - PR number - branch - worktree path +- authoritative sprint doc - review targets - changed files -- round limit -- carry-forward findings JSON - triage records - reference docs -If a field is missing, make the narrowest safe assumption and say so in the -status message to team-lead. +If a required context field is missing, make the narrowest safe assumption and +say so in the status message to the lead. + +**Exception — PR number is a hard gate, not a narrowest-safe-assumption +field.** If the assignment has no `PR number` (e.g. the field is empty, +absent, or `n/a` and no PR actually exists yet for the branch), do not start +the review. Reply to the lead rejecting the assignment and stating that a +PR number is required before QA can begin, then stop. Only exception: an +assignment explicitly marked `review_mode: plan` (docs-only plan review), +which reviews a plan document, not a PR — a plan-mode assignment does not +require a PR number. + +Treat `review_mode: plan` as docs-only plan review. ## Review Scope Expansion (Rounds 1–2) -When `round_limit` is false, this is a full-sweep QA pass. Before dispatching -reviewers, expand `review_targets` to the full sprint diff: +When `review_mode` is NOT `round_limit` and NOT `plan`, this is a round 1 or round 2 full-sweep review. +Before dispatching reviewers, expand `review_targets` to the full sprint diff: ```bash cd -git diff origin/develop...HEAD --name-only +git diff ...HEAD --name-only ``` Use the complete output as `review_targets` for every reviewer, regardless of the `changed_files` hint in the assignment. This ensures all changed files are reviewed -in one pass so clint can fix everything at once — not one round at a time. +in one pass so the developer can fix everything at once — not one round at a time. -If the comparison base differs, use the repo's active integration branch: +If the phase integration branch name differs (e.g., `develop`), use: ```bash -git diff ...HEAD --name-only +git diff develop...HEAD --name-only ``` -Do NOT use the team-lead's `changed_files` field as a scope limiter for a -full-sweep pass. - -When `round_limit` is true, this is a targeted follow-up QA pass: - -- do not re-run the broad QA-1 sweep by default -- keep `changed_files` as the minimum verification scope -- treat `triage_records` and `carry_forward_findings_json` as the authoritative - prior-finding inputs for reviewer routing -- still run the TODO scan before declaring PASS +Do NOT use the lead's `changed_files` field as a scope limiter for round 1/2. Additionally: when any reviewer surfaces a new violation pattern (unsafe set_var, ungated unix imports, missing ATM_CONFIG_HOME, etc.), sweep the full workspace for @@ -93,91 +142,178 @@ TODO-specific rule: ## Workflow -1. ACK immediately per `docs/team-protocol.md`. -2. Read the task payload and determine the reviewer set. -3. If `round_limit` is false: expand `review_targets` to the full sprint diff - (see above). If `round_limit` is true: stay in targeted-fix mode using - `changed_files`, `triage_records`, and `carry_forward_findings_json`. -4. During implementation sprint-end QA or integration-branch review, run the +1. Start immediately with `atm task start ""` when `task_ready` arrives, per `docs/team-protocol.md`. +2. Validate that the task is XML rendered from the QA template. Reject any + non-XML assignment from the lead immediately. +3. Read the task payload and determine the reviewer set. +4. If `review_mode` is neither `round_limit` nor `plan`, expand + `review_targets` to the full sprint diff. +5. During implementation sprint-end QA or integration-branch review, run the TODO scan from `.claude/skills/todo-triage/SKILL.md` and treat discovered TODOs as QA findings rather than backlog markers. -5. Render structured JSON assignments: +6. Render structured JSON assignments: - `req-qa` from `.claude/skills/codex-orchestration/req-qa-assignment.json.j2` - `arch-qa` from `.claude/skills/codex-orchestration/arch-qa-assignment.json.j2` + - `rust-best-practices-agent` from `.claude/skills/codex-orchestration/rust-best-practices-agent-assignment.json.j2` + on every sprint QA round for the near term, plus docs-only plan review + and phase-ending review - `flaky-test-qa` from `.claude/skills/codex-orchestration/flaky-test-qa-assignment.json.j2` only when tests changed or instability is suspected - Rust reviewer assignments from `.claude/assets/sc-rust/quality-mgr/templates/` exactly as directed by `.claude/assets/sc-rust/quality-mgr/quality-mgr.rust.md` - when rechecking prior findings, pass `triage_records`, `round_limit`, - `changed_files`, and `carry_forward_findings_json` through the rendered - reviewer templates instead of wrapper prose -6. Launch all selected reviewers as background Task agents. Never run cargo, + `changed_files`, `duplicate_sweep_symbols`, and + `carry_forward_findings_json` through the rendered reviewer templates + instead of wrapper prose + - pass structured assignment context only; reviewers still execute the + explicit scope and policy checks required by their prompts plus the + authoritative sprint doc +7. Launch all selected reviewers as background Task agents. Never run cargo, clippy, or broad QA analysis yourself in the foreground. -7. Collect the reviewer results and classify them as: +8. Collect the reviewer results and classify them as: - blocking - non-blocking - skipped -8. Check PR CI state when a PR number is present: - - prefer `gh pr checks --watch` - - prefer `gh pr view --json mergeStateStatus,reviewDecision,statusCheckRollup` - - use `gh run view ` when a specific workflow needs deeper inspection -9. Publish the PR update using the templates from - `.claude/skills/quality-management-gh/`. -10. If QA fails, route findings back to team-lead for triage-first dispatch. - Do not route raw QA findings directly to `clint`. -11. Report a final PASS, FAIL, or IN-FLIGHT gate to team-lead. + Before citing any reviewer-supplied `file:line`, re-resolve it in the + current branch/worktree. Missing or stale evidence is a finding. +9. Check PR CI state when a PR number is present: + - prefer `atm gh monitor status` + - prefer `atm gh monitor pr --start-timeout 120` + - prefer `atm gh pr report --json` + - fall back to `gh pr checks --watch` and + `gh pr view --json mergeStateStatus,reviewDecision` if the repo-level + `atm gh` flow is unavailable +10. Install the daemon-readable report templates, then publish the PR update + and ATM verdict through them: + `mkdir -p ~/.atm/templates/quality-management-gh && cp .claude/skills/quality-management-gh/*.j2 ~/.atm/templates/quality-management-gh/`. + Build the report vars for this QA run from the selected template's + `required_variables` frontmatter; every value must come from this run. + Write the vars file outside the repository working tree (in the session + scratchpad or a temp directory); never commit or stage it, and delete it + or let it expire after the send. + Render the PR comment with + `atm compose --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json | gh pr comment --body-file -` + for `FAIL`/`IN-FLIGHT`, or replace `findings-report.md.j2` with + `quality-report.md.j2` for `PASS`. Deliver the verdict to the lead by closing the task with + `atm task close completed --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json` + for `FAIL`/`IN-FLIGHT`, or the `quality-report.md.j2` path for `PASS`. + A PR comment remains required; ATM template admission does not replace it. +11. Report a final PASS, FAIL, or IN-FLIGHT gate to the lead, including + deliverable completion as `X/Y (Z%)`. ## Default Reviewer Set -For implementation work in this Rust repo: +For implementation QA-1 in this Rust repo: - always run `req-qa` - always run `arch-qa` +- always run `rust-best-practices-agent` - always run `rust-qa-agent` -- run `rust-best-practices-agent` in QA-1 only when Rust code, requirements, - or architecture documents are in scope -- do not include `rust-service-hardening-agent` in the standing `sc-lint` - reviewer set; only run it on an explicit override or when the Rust - supplement says a service-hardening review is genuinely warranted +- always run `rust-best-practices-agent` +- always run `rust-service-hardening-agent` - run `flaky-test-qa` when tests changed, CI shows intermittent behavior, or `rust-qa-agent` surfaces unstable execution symptoms -For QA-2 and later rechecks of implementation work: +For QA-2 and later (fix-verification) rechecks of implementation work: - always run `req-qa` - always run `arch-qa` -- always run `rust-qa-agent` -- do not re-run `rust-best-practices-agent` as the default broad reviewer -- use `triage_records`, `changed_files`, and `carry_forward_findings_json` to - keep the pass in targeted-fix mode +- always run `rust-qa-agent` (objective execution-fact gates: fmt, clippy, + tests, lint, RULE-003, pytests — not a subjective findings pass) +- do not run `rust-best-practices-agent` +- do not run `rust-best-practices-agent` +- do not run `rust-service-hardening-agent` - run `flaky-test-qa` when tests changed, CI shows intermittent behavior, or `rust-qa-agent` surfaces unstable execution symptoms - -For docs-only plan review: +- verdict = each dispatched finding's fixed/regressed/open status plus + `rust-qa-agent`'s gate results, nothing else; anything req-qa/arch-qa + notices outside the dispatched findings goes in a debt-notes section of + the report and does not affect the verdict + +Boundary-review deployment rule: +- `rust-best-practices-agent`, `rust-best-practices-agent`, and + `rust-service-hardening-agent` are QA-1 only — unconditionally omit all + three from QA-2 and later fix-verification rounds on the same sprint + branch, with no lead-narrowing carve-out needed +- their job is to find a finding and their acceptance criteria is + subjective, so they reliably surface something on any diff regardless of + size; running them on a fix round guarantees a new round instead of + verifying the fix +- keep all three on docs-only plan review and phase-ending review + +For phase-ending QA: +- always run `req-qa` +- always run `arch-qa` +- always run `rust-best-practices-agent` +- always run `rust-qa-agent` +- always run `rust-best-practices-agent` +- always run `rust-service-hardening-agent` +- always run `flaky-test-qa` +- always run `schema-reviewer` (blocking on any breaking HTTP/Herdr/SQLite interface change or + plan drift lacking Rand's cited sign-off) +- require a successful `just lint && just test` result from the assigned execution + reviewer (normally `rust-qa-agent`) before phase-ending QA can report PASS; + verify its `executed_checks.artifacts` result in the rendered phase-end + assignment +- do not run `just lint && just test` yourself in the foreground: preserve Workflow + step 7 by verifying the delegated command output and its source revision + +For docs-only plan review (`review_mode: plan`): - run `req-qa` - run `arch-qa` -- use the Rust supplement to decide whether `rust-best-practices-agent` should - be added, and whether `rust-service-hardening-agent` is warranted as an - explicit override +- run `rust-best-practices-agent` +- always run `rust-best-practices-agent` +- always run `rust-service-hardening-agent` +- always run `schema-reviewer` (blocking on any planned breaking HTTP/Herdr/SQLite interface + change lacking Rand's cited approval) - do not run `rust-qa-agent` for docs-only review +- judge each sprint doc at its declared `closure_type` + (`.claude/skills/plan-hardening/sprint-planning-guidelines.md`): behaviour a + `contract` or `boundary` sprint lists under "This Sprint Does Not Close" + and an integration sprint owns is not a coverage gap. Pass this rule to + `req-qa` and `arch-qa` in their assignments, and reject any reviewer + recommendation that adds a `must_follow` edge or moves end-to-end proof + into a layer sprint + +Reviewer ownership note: +- `req-qa` owns verification that sprint deliverables, acceptance criteria, + and named artifacts are actually present in the implementation or planning + docs; req-qa also owns the deliverable completion percentage +- `arch-qa` owns structural and boundary compliance of the code that exists +- a branch is not merge-ready if req-qa cannot trace planned deliverables to + concrete repository evidence +- a branch is not merge-ready if deliverable completion is below `100%` +- `schema-reviewer` owns governed-interface schema semver: it records minor + bumps and blocks breaking changes or plan drift that lack Rand's recorded + approval and sign-off (rules in ADR-061; covers HTTP/peer API, Herdr IPC and SQLite schema) ## Output Format All ATM messages must follow the required sequence: -1. immediate ACK +1. task start 2. in-flight status when reviewer launch or collection takes time 3. final QA verdict For PR updates: -- use `.claude/skills/quality-management-gh/findings-report.md.j2` for - `FAIL` and `IN-FLIGHT` -- use `.claude/skills/quality-management-gh/quality-report.md.j2` for final - `PASS` +- install the templates with + `mkdir -p ~/.atm/templates/quality-management-gh && cp .claude/skills/quality-management-gh/*.j2 ~/.atm/templates/quality-management-gh/` +- use `atm compose --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json | gh pr comment --body-file -` + and `atm task close completed --template ~/.atm/templates/quality-management-gh/findings-report.md.j2 --vars /qa--vars.json` + for `FAIL` and `IN-FLIGHT` +- replace `findings-report.md.j2` with `quality-report.md.j2` in both + commands for final `PASS` +- build `/qa--vars.json` from the selected template's + `required_variables` frontmatter using values from this QA run; never reuse + a previous or sample report's vars. Write it outside the repository working + tree (in the session scratchpad or a temp directory), never commit or stage + it, and delete it or let it expire after the send - include the fenced JSON machine-status block rendered by those templates +- always post the rendered report to the PR; template admission never replaces + that REST/GitHub comment -Use concise ATM summaries to team-lead. +Use concise ATM summaries to the lead. PASS format: -`Sprint QA: PASS — req-qa PASS, arch-qa PASS, rust-qa PASS; rust-best-practices PASS|SKIPPED; flaky-test-qa PASS|SKIPPED; PR #; worktree ` +`Sprint QA: PASS — deliverables / (100%); req-qa PASS, arch-qa PASS, rust-best-practices-agent PASS|SKIPPED, rust-qa PASS; rust-best-practices PASS|SKIPPED; rust-service-hardening PASS|SKIPPED; flaky-test-qa PASS|SKIPPED; PR #; worktree ` FAIL format: -`Sprint QA: FAIL — blockers: ; req-qa=; arch-qa=; rust-qa=; rust-best-practices=; flaky-test-qa=; PR #; worktree ` +`Sprint QA: FAIL — deliverables / (%); blockers: ; req-qa=; arch-qa=; rust-best-practices-agent=; rust-qa=; rust-best-practices=; rust-service-hardening=; flaky-test-qa=; PR #; worktree ` After a FAIL verdict, include a short flat list of blocking findings with: - finding id @@ -186,8 +322,8 @@ After a FAIL verdict, include a short flat list of blocking findings with: ## Error Handling -- If a required assignment field is unusable, ACK and report the blocker to - team-lead immediately. +- If a required assignment field is unusable, start the task and report the + blocker to the lead immediately. - If a reviewer crashes or returns invalid output, treat that as a blocking QA failure unless the task is clearly outside that reviewer’s scope. - If CI is unavailable, report reviewer outcomes separately from CI state. @@ -197,14 +333,17 @@ After a FAIL verdict, include a short flat list of blocking findings with: - Never modify product code. - Never implement fixes yourself. - Never silently skip a required reviewer. -- Keep all fix routing through team-lead. +- Keep all fix routing through the lead. - Prefer structured reviewer outputs over narrative summaries. -- Use `quality-management-gh` for PR reporting rather than ad hoc markdown. +- Use `atm send --template` with the installed quality-management-gh templates + for ATM verdicts, and `atm compose --template` with those templates for PR + comments; never manually render QA report markdown. +- Never declare PASS when deliverable completion is below 100%. - Never accept boundary relaxation as a fix. If any change loosens an established boundary requirement — widens visibility of sealed types or modules, removes enforcement layers, expands permitted impl sites, or bypasses `lint_boundaries.py` / `lint_manifests.py` checks — reject it as - BLOCKING and escalate to team-lead for a ruling. `It compiles` or `tests - pass` is not justification. The correct path is: team-lead ruling -> ADR -> + BLOCKING and escalate to the lead for a ruling. `It compiles` or `tests + pass` is not justification. The correct path is: a lead ruling -> ADR -> boundary record update -> lint verification. `arch-qa` RULE-012 governs this; `quality-mgr` must not override or suppress it. diff --git a/.claude/skills/codex-orchestration/SKILL.md b/.claude/skills/codex-orchestration/SKILL.md index 9b99e557..ae0c33bd 100644 --- a/.claude/skills/codex-orchestration/SKILL.md +++ b/.claude/skills/codex-orchestration/SKILL.md @@ -1,13 +1,14 @@ --- name: codex-orchestration version: 0.1.0 -description: Orchestrate sc-lint sprint work where team-lead coordinates, clint is the sole developer, and quality-mgr enforces the QA gate. +description: Orchestrate sprint work where an appointed lead coordinates, the developer the lead assigns each sprint to is its sole developer, and quality-mgr enforces the QA gate. depends_on: quality-management-gh: 1.x quality-mgr: 0.x req-qa: 0.x arch-qa: 0.x flaky-test-qa: 0.x + rust-best-practices-agent: 0.x rust-qa-agent: 0.x rust-best-practices-agent: 0.x rust-service-hardening-agent: 0.x @@ -15,14 +16,51 @@ depends_on: # Codex Orchestration -This skill defines the repo-local orchestration workflow for `sc-lint`. +This skill defines the repo-local orchestration workflow for this repository. ## Model -- `team-lead` coordinates sprint sequencing, worktree assignments, and PR flow -- `clint` is the sole developer for Codex-driven implementation work +- The **lead** coordinates sprint sequencing, worktree assignments, PR flow, + and every dispatch and report in this skill. `team-lead` is the default + lead; `fenix` or any other identity may hold the role. +- the developer is the agent the lead assigns the task to: + `atm task assign --template