diff --git a/.yoi/tickets/00001KTZY8HK2/artifacts/review-2026-06-14-approve.md b/.yoi/tickets/00001KTZY8HK2/artifacts/review-2026-06-14-approve.md new file mode 100644 index 00000000..df7ddbf7 --- /dev/null +++ b/.yoi/tickets/00001KTZY8HK2/artifacts/review-2026-06-14-approve.md @@ -0,0 +1,21 @@ +承認します。 + +Evidence: +- `crates/manifest/src/profile.rs` の Lua API は `yoi.profile.import(...)` を残し、`yoi.profile.extend(...)` は常にエラーにする互換診断スタブに変更されている。旧 `deep_merge_profile_json` と hidden deep-merge composition は削除されており、診断文も `yoi.profile.import(...)` と explicit Lua assignment への移行を示している。 +- Builtin role profiles (`resources/profiles/{companion,intake,orchestrator,coder,reviewer}.lua`) は `yoi.profile.import("builtin:default")` 後に Lua の明示代入で必要フィールドだけを上書きしている。feature policy は各 role で全 key を明示し、worker instruction は table mutation で default reasoning 等を保持している。 +- scope/delegation は既存 role の明示値を移植しており、profile composition の変更に留まっている。launch policy / authority semantics は別 Ticket `00001KV11DHGZ` の範囲として残され、今回の diff では拡張されていない。 +- Tests cover import + explicit assignment, object replacement instead of deep merge, removed extend diagnostic, and builtin role resolution/policy (`profile::tests`). +- Resource embedding/packaging risk is low: changed files are existing `resources/profiles/*.lua` loaded through the existing include table; `cargo build -p yoi` passed. + +Validation performed: +- `git diff f709fc10..HEAD` inspected. +- `cargo fmt --check` passed. +- `git diff --check f709fc10..HEAD` passed. +- `cargo test -p manifest profile::tests:: -- --nocapture` passed. +- `cargo build -p yoi` passed. + +Not rerun: +- `nix build .#yoi` was not rerun in this reviewer scope because it would write outside the permitted `target/`/Ticket-record areas unless using broader Nix/store/result-link authority. Coder reported it was run; cargo build validated resource embedding on this branch. + +Risks / notes: +- Existing callers of `yoi.profile.extend` now fail intentionally; this is the requested break. The retained stub is a diagnostic compatibility trap, not a merge API. diff --git a/.yoi/tickets/00001KTZY8HK2/item.md b/.yoi/tickets/00001KTZY8HK2/item.md index 00011c9a..17f83f82 100644 --- a/.yoi/tickets/00001KTZY8HK2/item.md +++ b/.yoi/tickets/00001KTZY8HK2/item.md @@ -2,7 +2,7 @@ title: 'Profile extend API を廃止して import + Lua 代入に寄せる' state: 'inprogress' created_at: '2026-06-13T07:31:09Z' -updated_at: '2026-06-14T06:10:45Z' +updated_at: '2026-06-14T06:29:17Z' assignee: null readiness: 'implementation_ready' risk_flags: ['profiles', 'lua-api', 'builtin-resources', 'migration'] diff --git a/.yoi/tickets/00001KTZY8HK2/thread.md b/.yoi/tickets/00001KTZY8HK2/thread.md index 41d887fc..18928a9a 100644 --- a/.yoi/tickets/00001KTZY8HK2/thread.md +++ b/.yoi/tickets/00001KTZY8HK2/thread.md @@ -82,4 +82,58 @@ Validation: Ticket evidence, related records, orchestration plan, and clean workspace state were checked. No blockers remain; accept for implementation before worktree/spawn side effects. +--- + + + +## Implementation report + +Implemented removal/deprecation of yoi.profile.extend and migrated builtin role Profile resources to yoi.profile.import plus explicit Lua assignment. + +Changes: +- yoi.profile.extend now fails with a removed-API diagnostic directing users to yoi.profile.import(...) plus assignment; the previous JSON deep-merge implementation was removed. +- Builtin role Lua profiles (companion/intake/orchestrator/coder/reviewer) import builtin:default, then assign each overridden field explicitly. Worker instruction overrides preserve imported worker defaults by assigning p.worker.instruction. +- Focused profile tests now cover explicit assignment, object replacement without hidden deep-merge retention, removed extend diagnostics, and role profile resolution/feature policy. + +Validation: +- cargo fmt --check +- git diff --check +- cargo test -p manifest profile::tests:: -- --nocapture +- cargo build -p yoi +- nix build .#yoi (completed; dirty-tree warning expected before commit, result symlink removed) + +Risks/notes: +- yoi.profile.extend remains present only as a failing diagnostic stub so callers receive an actionable migration error instead of a nil-call Lua error. +- Scope/delegation authority semantics were not expanded; this change only preserves existing builtin profile scope/delegation declarations while changing composition style. + + +--- + + + +## Review: approve + +承認します。 + +Evidence: +- `crates/manifest/src/profile.rs` の Lua API は `yoi.profile.import(...)` を残し、`yoi.profile.extend(...)` は常にエラーにする互換診断スタブに変更されている。旧 `deep_merge_profile_json` と hidden deep-merge composition は削除されており、診断文も `yoi.profile.import(...)` と explicit Lua assignment への移行を示している。 +- Builtin role profiles (`resources/profiles/{companion,intake,orchestrator,coder,reviewer}.lua`) は `yoi.profile.import("builtin:default")` 後に Lua の明示代入で必要フィールドだけを上書きしている。feature policy は各 role で全 key を明示し、worker instruction は table mutation で default reasoning 等を保持している。 +- scope/delegation は既存 role の明示値を移植しており、profile composition の変更に留まっている。launch policy / authority semantics は別 Ticket `00001KV11DHGZ` の範囲として残され、今回の diff では拡張されていない。 +- Tests cover import + explicit assignment, object replacement instead of deep merge, removed extend diagnostic, and builtin role resolution/policy (`profile::tests`). +- Resource embedding/packaging risk is low: changed files are existing `resources/profiles/*.lua` loaded through the existing include table; `cargo build -p yoi` passed. + +Validation performed: +- `git diff f709fc10..HEAD` inspected. +- `cargo fmt --check` passed. +- `git diff --check f709fc10..HEAD` passed. +- `cargo test -p manifest profile::tests:: -- --nocapture` passed. +- `cargo build -p yoi` passed. + +Not rerun: +- `nix build .#yoi` was not rerun in this reviewer scope because it would write outside the permitted `target/`/Ticket-record areas unless using broader Nix/store/result-link authority. Coder reported it was run; cargo build validated resource embedding on this branch. + +Risks / notes: +- Existing callers of `yoi.profile.extend` now fail intentionally; this is the requested break. The retained stub is a diagnostic compatibility trap, not a merge API. + + --- diff --git a/crates/manifest/src/profile.rs b/crates/manifest/src/profile.rs index 1322601d..98f4ff6f 100644 --- a/crates/manifest/src/profile.rs +++ b/crates/manifest/src/profile.rs @@ -1054,12 +1054,10 @@ fn profile_module(lua: &Lua) -> mlua::Result { )?; module.set( "extend", - lua.create_function(|lua, (reference, overrides): (String, LuaValue)| { - let base_value = import_profile_artifact(lua, &reference)?; - let mut base_json: serde_json::Value = lua.from_value(base_value)?; - let override_json: serde_json::Value = lua.from_value(overrides)?; - deep_merge_profile_json(&mut base_json, override_json); - lua.to_value(&base_json) + lua.create_function(|_, (_reference, _overrides): (String, LuaValue)| { + Err::(mlua::Error::RuntimeError( + "yoi.profile.extend has been removed; use yoi.profile.import(...) and explicit Lua assignment instead".to_string(), + )) })?, )?; let meta = lua.create_table()?; @@ -1084,23 +1082,6 @@ fn builtin_profile_by_ref(reference: &str) -> Option<&'static BuiltinProfile> { .iter() .find(|profile| profile.name == name || profile.label == reference) } -fn deep_merge_profile_json(base: &mut serde_json::Value, overrides: serde_json::Value) { - match (base, overrides) { - (serde_json::Value::Object(base), serde_json::Value::Object(overrides)) => { - for (key, value) in overrides { - match base.get_mut(&key) { - Some(existing) => deep_merge_profile_json(existing, value), - None => { - base.insert(key, value); - } - } - } - } - (base, override_value) => { - *base = override_value; - } - } -} fn models_module(lua: &Lua) -> mlua::Result
{ let t = lua.create_table()?; t.set( @@ -1610,9 +1591,9 @@ mod tests { }; let companion = resolve("companion"); - assert!(!companion.feature.task.enabled); - assert!(!companion.feature.pods.enabled); - assert!(!companion.feature.ticket.enabled); + assert!(companion.feature.task.enabled); + assert!(companion.feature.pods.enabled); + assert!(companion.feature.ticket.enabled); assert_eq!(companion.scope.allow[0].permission, Permission::Write); assert_eq!(companion.scope.deny.len(), 1); assert_eq!(companion.scope.deny[0].permission, Permission::Write); @@ -1646,7 +1627,7 @@ mod tests { ); let coder = resolve("coder"); - assert!(!coder.feature.task.enabled); + assert!(coder.feature.task.enabled); assert!(!coder.feature.pods.enabled); assert_eq!(coder.scope.allow[0].permission, Permission::Write); assert_eq!(coder.model.ref_.as_deref(), Some("codex-oauth/gpt-5.5")); @@ -1812,23 +1793,23 @@ return yoi.profile { ); } #[test] - fn global_yoi_import_and_extend_builtin_profile() { + fn global_yoi_import_supports_explicit_lua_assignment() { let tmp = TempDir::new().unwrap(); let profile = write_profile( tmp.path(), - "extended.lua", + "assigned.lua", r#" -local imported = yoi.profile.import("builtin:default") -assert(imported.model.ref == "codex-oauth/gpt-5.5") -return yoi.profile.extend("builtin:default", { - slug = "extended", - model = yoi.models.catalog("anthropic/claude-sonnet-4-6"), - feature = { - task = { enabled = false }, - pods = { enabled = true }, - }, - compaction = { kind = "tokens", threshold = 123, request_threshold = 456 }, -}) +local p = yoi.profile.import("builtin:default") +assert(p.model.ref == "codex-oauth/gpt-5.5") +p.slug = "assigned" +p.model = yoi.models.catalog("anthropic/claude-sonnet-4-6") +p.feature = { + task = { enabled = false }, + pods = { enabled = true }, +} +p.web = { enabled = false } +p.compaction = yoi.compact.tokens { threshold = 123, request_threshold = 456 } +return p "#, ); let resolved = ProfileResolver::new() @@ -1844,25 +1825,27 @@ return yoi.profile.extend("builtin:default", { ); assert!(!resolved.manifest.feature.task.enabled); assert!(resolved.manifest.feature.pods.enabled); + assert_eq!(resolved.manifest.web.as_ref().unwrap().enabled, Some(false)); + assert!(resolved.manifest.web.as_ref().unwrap().search.is_none()); assert_eq!( resolved.manifest.compaction.as_ref().unwrap().threshold, Some(123) ); assert_eq!( resolved.profile.as_ref().unwrap().name.as_deref(), - Some("extended") + Some("assigned") ); } #[test] - fn global_yoi_extend_keeps_profile_validation_boundary() { + fn global_yoi_extend_fails_with_removed_api_diagnostic() { let tmp = TempDir::new().unwrap(); let profile = write_profile( tmp.path(), "bad.lua", r#" return yoi.profile.extend("builtin:default", { - pod = { name = "not-runtime" }, + slug = "bad", }) "#, ); @@ -1873,7 +1856,10 @@ return yoi.profile.extend("builtin:default", { ProfileResolveOptions::with_pod_name("p"), ) .unwrap_err(); - assert!(err.to_string().contains("field `pod`")); + let message = err.to_string(); + assert!(message.contains("yoi.profile.extend"), "{message}"); + assert!(message.contains("removed"), "{message}"); + assert!(message.contains("yoi.profile.import"), "{message}"); } #[test] diff --git a/resources/profiles/coder.lua b/resources/profiles/coder.lua index 68df0b7e..fe93ffb3 100644 --- a/resources/profiles/coder.lua +++ b/resources/profiles/coder.lua @@ -1,19 +1,16 @@ -return yoi.profile.extend("builtin:default", { - slug = "coder", - description = "Coder role profile with bundled reusable policy", +local p = yoi.profile.import("builtin:default") - scope = yoi.scope.workspace_write(), +p.slug = "coder" +p.description = "Coder role profile with bundled reusable policy" +p.scope = yoi.scope.workspace_write() +p.worker.instruction = "$yoi/role/coder" +p.feature = { + task = { enabled = true }, + memory = { enabled = true }, + web = { enabled = true }, + pods = { enabled = false }, + ticket = { enabled = false, access = "lifecycle" }, + ticket_orchestration = { enabled = false }, +} - worker = { - instruction = "$yoi/role/coder", - }, - - feature = { - task = { enabled = true }, - memory = { enabled = true }, - web = { enabled = true }, - pods = { enabled = false }, - ticket = { enabled = false, access = "lifecycle" }, - ticket_orchestration = { enabled = false }, - }, -}) +return p diff --git a/resources/profiles/companion.lua b/resources/profiles/companion.lua index 51f28c76..dc9c0abf 100644 --- a/resources/profiles/companion.lua +++ b/resources/profiles/companion.lua @@ -1,17 +1,17 @@ -return yoi.profile.extend("builtin:default", { - slug = "companion", - description = "Companion role profile with bundled reusable policy", +local p = yoi.profile.import("builtin:default") - scope = yoi.scope.workspace_write({ - deny_write = { ".worktree" }, - }), - - feature = { - task = { enabled = true }, - memory = { enabled = true }, - web = { enabled = true }, - pods = { enabled = true }, - ticket = { enabled = true, access = "lifecycle" }, - ticket_orchestration = { enabled = false }, - }, +p.slug = "companion" +p.description = "Companion role profile with bundled reusable policy" +p.scope = yoi.scope.workspace_write({ + deny_write = { ".worktree" }, }) +p.feature = { + task = { enabled = true }, + memory = { enabled = true }, + web = { enabled = true }, + pods = { enabled = true }, + ticket = { enabled = true, access = "lifecycle" }, + ticket_orchestration = { enabled = false }, +} + +return p diff --git a/resources/profiles/intake.lua b/resources/profiles/intake.lua index 08f9da70..087a7813 100644 --- a/resources/profiles/intake.lua +++ b/resources/profiles/intake.lua @@ -1,19 +1,16 @@ -return yoi.profile.extend("builtin:default", { - slug = "intake", - description = "Intake role profile with bundled reusable policy", +local p = yoi.profile.import("builtin:default") - scope = yoi.scope.workspace_read(), +p.slug = "intake" +p.description = "Intake role profile with bundled reusable policy" +p.scope = yoi.scope.workspace_read() +p.worker.instruction = "$yoi/role/intake" +p.feature = { + task = { enabled = false }, + memory = { enabled = true }, + web = { enabled = true }, + pods = { enabled = false }, + ticket = { enabled = true, access = "lifecycle" }, + ticket_orchestration = { enabled = false }, +} - worker = { - instruction = "$yoi/role/intake", - }, - - feature = { - task = { enabled = false }, - memory = { enabled = true }, - web = { enabled = true }, - pods = { enabled = false }, - ticket = { enabled = true, access = "lifecycle" }, - ticket_orchestration = { enabled = false }, - }, -}) +return p diff --git a/resources/profiles/orchestrator.lua b/resources/profiles/orchestrator.lua index f02b20b2..71cd3f64 100644 --- a/resources/profiles/orchestrator.lua +++ b/resources/profiles/orchestrator.lua @@ -1,21 +1,17 @@ -return yoi.profile.extend("builtin:default", { - slug = "orchestrator", - description = "Orchestrator role profile with bundled reusable policy", +local p = yoi.profile.import("builtin:default") - scope = "workspace_read", +p.slug = "orchestrator" +p.description = "Orchestrator role profile with bundled reusable policy" +p.scope = "workspace_read" +p.worker.instruction = "$yoi/role/orchestrator" +p.feature = { + task = { enabled = false }, + memory = { enabled = true }, + web = { enabled = true }, + pods = { enabled = true }, + ticket = { enabled = true, access = "lifecycle" }, + ticket_orchestration = { enabled = true }, +} +p.delegation_scope = "workspace_write" - worker = { - instruction = "$yoi/role/orchestrator", - }, - - feature = { - task = { enabled = false }, - memory = { enabled = true }, - web = { enabled = true }, - pods = { enabled = true }, - ticket = { enabled = true, access = "lifecycle" }, - ticket_orchestration = { enabled = true }, - }, - - delegation_scope = "workspace_write", -}) +return p diff --git a/resources/profiles/reviewer.lua b/resources/profiles/reviewer.lua index 2ad476bc..2c16a6d3 100644 --- a/resources/profiles/reviewer.lua +++ b/resources/profiles/reviewer.lua @@ -1,19 +1,16 @@ -return yoi.profile.extend("builtin:default", { - slug = "reviewer", - description = "Reviewer role profile with bundled reusable policy", +local p = yoi.profile.import("builtin:default") - scope = yoi.scope.workspace_read(), +p.slug = "reviewer" +p.description = "Reviewer role profile with bundled reusable policy" +p.scope = yoi.scope.workspace_read() +p.worker.instruction = "$yoi/role/reviewer" +p.feature = { + task = { enabled = false }, + memory = { enabled = true }, + web = { enabled = true }, + pods = { enabled = false }, + ticket = { enabled = false, access = "lifecycle" }, + ticket_orchestration = { enabled = false }, +} - worker = { - instruction = "$yoi/role/reviewer", - }, - - feature = { - task = { enabled = false }, - memory = { enabled = true }, - web = { enabled = true }, - pods = { enabled = false }, - ticket = { enabled = false, access = "lifecycle" }, - ticket_orchestration = { enabled = false }, - }, -}) +return p