diff --git a/work-items/open/20260604-234844-feature-api-authority-separation/artifacts/implementation-report.md b/work-items/open/20260604-234844-feature-api-authority-separation/artifacts/implementation-report.md new file mode 100644 index 00000000..10113feb --- /dev/null +++ b/work-items/open/20260604-234844-feature-api-authority-separation/artifacts/implementation-report.md @@ -0,0 +1,76 @@ +# Implementation report: feature-api-authority-separation + +## Worktree / branch + +- Worktree: `/home/hare/Projects/yoi/.worktree/feature-api-authority-separation` +- Branch: `work/feature-api-authority-separation` + +## Commit + +- `4fc361f refactor: name feature host authorities explicitly` + +## Summary + +Clarified the `pod::feature` authority boundary by renaming the generic authority API surface to explicit host-authority terminology. This keeps feature contribution declarations separate from host-mediated capability grants and prepares the API for later Ticket built-in tools without framing internal built-ins as external plugin package grants. + +## Exact renames + +- `AuthorityRequest` -> `HostAuthorityRequest` +- `AuthorityGrantSet` -> `HostAuthorityGrantSet` +- `AuthorityDenial` -> `HostAuthorityDenial` +- `FeatureDescriptor::requested_authorities` -> `requested_host_authorities` +- `FeatureDescriptor::with_authority` -> `with_host_authority` +- `ToolContribution::required_authorities` -> `required_host_authorities` +- `ToolContribution::with_required_authorities` -> `with_required_host_authorities` +- `FeatureInstallReport::granted_authorities` -> `host_authority_grants` +- `FeatureInstallContext::grants()` -> `host_authority_grants()` +- `FeatureInstallError::AuthorityDenied` -> `HostAuthorityDenied` +- Internal helpers/diagnostics now use host-authority terminology where applicable. + +## Changed files + +- `crates/pod/src/feature.rs` + +## Behavior + +Preserved: + +- descriptor-first validation; +- duplicate tool rejection; +- undeclared contribution rejection; +- missing required host authority install failure; +- built-in Task feature behavior; +- contribution-only built-in feature installation without host authority grants. + +Added/updated tests and comments to make explicit that contributing a tool/hook/background/service descriptor is not itself a host authority grant, while per-tool host authority requirements still require a corresponding granted requested host authority. + +## Validation + +Coder-reported validation passed: + +- `cargo test -p pod feature --lib` +- `cargo test -p pod task --lib` +- `cargo test -p pod --lib` +- `cargo test -p llm-worker --lib` +- `cargo check --workspace --all-targets` +- `cargo fmt --check` +- `git diff --check` +- `./tickets.sh doctor` +- `nix build .#yoi --no-link` + +Reviewer-rerun validation passed: + +- `git diff --check develop...HEAD` +- `cargo test -p pod feature --lib` + +## Review status + +External sibling reviewer approved with no blockers and no required non-blockers before merge. + +## Unresolved risks / follow-ups + +The existing `HostAuthorityGrantSet::grant_all(&descriptor.requested_host_authorities)` behavior remains a builtin-only scaffold, not a real external plugin approval resolver. This is unchanged and explicitly outside this ticket's scope. + +## Ready for merge + +Yes. This clears the API naming prerequisite for `ticket-built-in-feature-tools`. diff --git a/work-items/open/20260604-234844-feature-api-authority-separation/artifacts/review.md b/work-items/open/20260604-234844-feature-api-authority-separation/artifacts/review.md new file mode 100644 index 00000000..f8d0284e --- /dev/null +++ b/work-items/open/20260604-234844-feature-api-authority-separation/artifacts/review.md @@ -0,0 +1,69 @@ +# External review: feature-api-authority-separation + +## 1. Result + +approve + +## 2. Summary of implementation + +The implementation is a focused `pod::feature` API cleanup in `crates/pod/src/feature.rs` only. It renames the generic authority API surface to explicit host-authority naming: + +- `AuthorityRequest` -> `HostAuthorityRequest` +- `AuthorityGrantSet` -> `HostAuthorityGrantSet` +- `AuthorityDenial` -> `HostAuthorityDenial` +- `FeatureDescriptor::requested_authorities` -> `requested_host_authorities` +- `FeatureDescriptor::with_authority` -> `with_host_authority` +- `ToolContribution::required_authorities` / builder -> `required_host_authorities` / `with_required_host_authorities` +- `FeatureInstallReport::granted_authorities` -> `host_authority_grants` +- `FeatureInstallContext::grants()` -> `host_authority_grants()` +- `FeatureInstallError::AuthorityDenied` -> `HostAuthorityDenied` + +The code comments and tests now describe contribution declarations as descriptor-approved contributions, not host authority grants. A new focused test covers a tool that needs a host authority: contribution declaration alone is not enough, while an explicit requested host authority grants the install-time host-authority requirement under the current grant-all scaffold. + +## 3. Requirement-by-requirement assessment + +- Generic authority ambiguity removed or clarified: satisfied. The changed source no longer exposes the generic `AuthorityRequest`, `AuthorityGrantSet`, `AuthorityDenial`, `requested_authorities`, `required_authorities`, `granted_authorities`, or `grants()` names in the live Rust API; all are explicitly host-authority named. +- Contribution declarations remain separate from host authority grants: satisfied. Tool/hook/background/service declarations remain descriptor contribution data, while host authorities are carried through `requested_host_authorities` and `host_authority_grants`. +- Built-in contribution registration does not require host authority merely to contribute descriptors: satisfied. `ToolContribution::new` defaults to no `required_host_authorities`, background tasks/services remain descriptor/report contributions, and Task installs with empty host-authority grants. +- Missing required host authority still fails feature installation where appropriate: satisfied for the current install surface. `ToolContribution::with_required_host_authorities` still rejects install when the corresponding host authority is absent from the grant set, and the new test covers this path. +- Descriptor-first validation and duplicate tool rejection preserved: satisfied. The existing undeclared contribution, tool-name mismatch, duplicate tool, background/service declaration, and worker materialization tests remain in place and passed in focused validation. +- Built-in Task feature behavior unchanged: satisfied by diff scope and focused tests. Task descriptor/install behavior still reports no host authorities and installs the same Task tool/hook set. +- No unrelated scope expansion: satisfied. The diff is limited to `crates/pod/src/feature.rs`; it does not introduce Ticket tools, Ticket backend authority grants, plugin loading, MCP, WASM/sandbox runtime, approval protocol, crate extraction, Hook behavior changes, Task behavior changes, or broad refactors. +- Tests/docs/comments reflect the distinction: satisfied. Public comments and test names/assertions now consistently use host-authority terminology for this API surface, and tests explicitly cover contribution-only built-ins and host-authority-gated tools. +- Validation sufficient: satisfied for this review. I reran focused validation and inspected the diff/source for the listed requirements. + +## 4. Blockers + +None. + +## 5. Non-blockers / follow-ups + +None required before merge. + +Notes for later external-plugin work: + +- `HostAuthorityGrantSet::grant_all(&descriptor.requested_host_authorities)` remains the existing builtin-only scaffold, not a real host policy/user approval resolver. This is unchanged and within this ticket's non-goals, but it must be replaced before enabling untrusted external plugins. +- `HostAuthorityRequest::required` versus `optional` remains future-facing under the current grant-all scaffold. No behavior change is introduced here. + +## 6. Validation assessed or rerun + +Rerun from `/home/hare/Projects/yoi/.worktree/feature-api-authority-separation`: + +- `git diff --check develop...HEAD` — passed. +- `cargo test -p pod feature --lib` — passed: 33 passed, 0 failed, 252 filtered out. + +Additional inspection: + +- Reviewed ticket and delegation intent. +- Reviewed `git diff develop...HEAD`. +- Confirmed the implementation diff touches only `crates/pod/src/feature.rs`. +- Searched Rust sources for the old generic authority names and found no live-code API leftovers, only new host-authority names or historical work-item text outside source. +- Checked worktree status after validation; no tracked changes. + +Not rerun: + +- Full workspace checks, broader package tests, `./tickets.sh doctor`, and `nix build .#yoi --no-link` were not rerun for this focused external review. + +## 7. Residual risk + +Residual risk is low for this ticket. The change is mostly API naming plus tests, and focused validation passed. The main remaining risk is pre-existing: the feature registry still automatically grants requested host authorities, so it is not yet a real external-plugin authority enforcement layer. That risk is explicitly outside this ticket's implementation scope and does not block this host-authority naming clarification. diff --git a/work-items/open/20260604-234844-feature-api-authority-separation/item.md b/work-items/open/20260604-234844-feature-api-authority-separation/item.md index 0e1c8574..74dee034 100644 --- a/work-items/open/20260604-234844-feature-api-authority-separation/item.md +++ b/work-items/open/20260604-234844-feature-api-authority-separation/item.md @@ -7,7 +7,7 @@ kind: task priority: P1 labels: [plugin, feature-registry, permissions, architecture] created_at: 2026-06-04T23:48:44Z -updated_at: 2026-06-05T04:54:33Z +updated_at: 2026-06-05T05:09:37Z assignee: null legacy_ticket: null --- diff --git a/work-items/open/20260604-234844-feature-api-authority-separation/thread.md b/work-items/open/20260604-234844-feature-api-authority-separation/thread.md index 597a1134..d74038d3 100644 --- a/work-items/open/20260604-234844-feature-api-authority-separation/thread.md +++ b/work-items/open/20260604-234844-feature-api-authority-separation/thread.md @@ -71,4 +71,165 @@ Detailed delegation intent is in `artifacts/delegation-intent.md`. Note: main workspace currently has an unrelated dirty `README.md` change. This feature work must not touch or commit that file. +--- + + + +## Review: approve + +# External review: feature-api-authority-separation + +## 1. Result + +approve + +## 2. Summary of implementation + +The implementation is a focused `pod::feature` API cleanup in `crates/pod/src/feature.rs` only. It renames the generic authority API surface to explicit host-authority naming: + +- `AuthorityRequest` -> `HostAuthorityRequest` +- `AuthorityGrantSet` -> `HostAuthorityGrantSet` +- `AuthorityDenial` -> `HostAuthorityDenial` +- `FeatureDescriptor::requested_authorities` -> `requested_host_authorities` +- `FeatureDescriptor::with_authority` -> `with_host_authority` +- `ToolContribution::required_authorities` / builder -> `required_host_authorities` / `with_required_host_authorities` +- `FeatureInstallReport::granted_authorities` -> `host_authority_grants` +- `FeatureInstallContext::grants()` -> `host_authority_grants()` +- `FeatureInstallError::AuthorityDenied` -> `HostAuthorityDenied` + +The code comments and tests now describe contribution declarations as descriptor-approved contributions, not host authority grants. A new focused test covers a tool that needs a host authority: contribution declaration alone is not enough, while an explicit requested host authority grants the install-time host-authority requirement under the current grant-all scaffold. + +## 3. Requirement-by-requirement assessment + +- Generic authority ambiguity removed or clarified: satisfied. The changed source no longer exposes the generic `AuthorityRequest`, `AuthorityGrantSet`, `AuthorityDenial`, `requested_authorities`, `required_authorities`, `granted_authorities`, or `grants()` names in the live Rust API; all are explicitly host-authority named. +- Contribution declarations remain separate from host authority grants: satisfied. Tool/hook/background/service declarations remain descriptor contribution data, while host authorities are carried through `requested_host_authorities` and `host_authority_grants`. +- Built-in contribution registration does not require host authority merely to contribute descriptors: satisfied. `ToolContribution::new` defaults to no `required_host_authorities`, background tasks/services remain descriptor/report contributions, and Task installs with empty host-authority grants. +- Missing required host authority still fails feature installation where appropriate: satisfied for the current install surface. `ToolContribution::with_required_host_authorities` still rejects install when the corresponding host authority is absent from the grant set, and the new test covers this path. +- Descriptor-first validation and duplicate tool rejection preserved: satisfied. The existing undeclared contribution, tool-name mismatch, duplicate tool, background/service declaration, and worker materialization tests remain in place and passed in focused validation. +- Built-in Task feature behavior unchanged: satisfied by diff scope and focused tests. Task descriptor/install behavior still reports no host authorities and installs the same Task tool/hook set. +- No unrelated scope expansion: satisfied. The diff is limited to `crates/pod/src/feature.rs`; it does not introduce Ticket tools, Ticket backend authority grants, plugin loading, MCP, WASM/sandbox runtime, approval protocol, crate extraction, Hook behavior changes, Task behavior changes, or broad refactors. +- Tests/docs/comments reflect the distinction: satisfied. Public comments and test names/assertions now consistently use host-authority terminology for this API surface, and tests explicitly cover contribution-only built-ins and host-authority-gated tools. +- Validation sufficient: satisfied for this review. I reran focused validation and inspected the diff/source for the listed requirements. + +## 4. Blockers + +None. + +## 5. Non-blockers / follow-ups + +None required before merge. + +Notes for later external-plugin work: + +- `HostAuthorityGrantSet::grant_all(&descriptor.requested_host_authorities)` remains the existing builtin-only scaffold, not a real host policy/user approval resolver. This is unchanged and within this ticket's non-goals, but it must be replaced before enabling untrusted external plugins. +- `HostAuthorityRequest::required` versus `optional` remains future-facing under the current grant-all scaffold. No behavior change is introduced here. + +## 6. Validation assessed or rerun + +Rerun from `/home/hare/Projects/yoi/.worktree/feature-api-authority-separation`: + +- `git diff --check develop...HEAD` — passed. +- `cargo test -p pod feature --lib` — passed: 33 passed, 0 failed, 252 filtered out. + +Additional inspection: + +- Reviewed ticket and delegation intent. +- Reviewed `git diff develop...HEAD`. +- Confirmed the implementation diff touches only `crates/pod/src/feature.rs`. +- Searched Rust sources for the old generic authority names and found no live-code API leftovers, only new host-authority names or historical work-item text outside source. +- Checked worktree status after validation; no tracked changes. + +Not rerun: + +- Full workspace checks, broader package tests, `./tickets.sh doctor`, and `nix build .#yoi --no-link` were not rerun for this focused external review. + +## 7. Residual risk + +Residual risk is low for this ticket. The change is mostly API naming plus tests, and focused validation passed. The main remaining risk is pre-existing: the feature registry still automatically grants requested host authorities, so it is not yet a real external-plugin authority enforcement layer. That risk is explicitly outside this ticket's implementation scope and does not block this host-authority naming clarification. + + +--- + + + +## Implementation report + +# Implementation report: feature-api-authority-separation + +## Worktree / branch + +- Worktree: `/home/hare/Projects/yoi/.worktree/feature-api-authority-separation` +- Branch: `work/feature-api-authority-separation` + +## Commit + +- `4fc361f refactor: name feature host authorities explicitly` + +## Summary + +Clarified the `pod::feature` authority boundary by renaming the generic authority API surface to explicit host-authority terminology. This keeps feature contribution declarations separate from host-mediated capability grants and prepares the API for later Ticket built-in tools without framing internal built-ins as external plugin package grants. + +## Exact renames + +- `AuthorityRequest` -> `HostAuthorityRequest` +- `AuthorityGrantSet` -> `HostAuthorityGrantSet` +- `AuthorityDenial` -> `HostAuthorityDenial` +- `FeatureDescriptor::requested_authorities` -> `requested_host_authorities` +- `FeatureDescriptor::with_authority` -> `with_host_authority` +- `ToolContribution::required_authorities` -> `required_host_authorities` +- `ToolContribution::with_required_authorities` -> `with_required_host_authorities` +- `FeatureInstallReport::granted_authorities` -> `host_authority_grants` +- `FeatureInstallContext::grants()` -> `host_authority_grants()` +- `FeatureInstallError::AuthorityDenied` -> `HostAuthorityDenied` +- Internal helpers/diagnostics now use host-authority terminology where applicable. + +## Changed files + +- `crates/pod/src/feature.rs` + +## Behavior + +Preserved: + +- descriptor-first validation; +- duplicate tool rejection; +- undeclared contribution rejection; +- missing required host authority install failure; +- built-in Task feature behavior; +- contribution-only built-in feature installation without host authority grants. + +Added/updated tests and comments to make explicit that contributing a tool/hook/background/service descriptor is not itself a host authority grant, while per-tool host authority requirements still require a corresponding granted requested host authority. + +## Validation + +Coder-reported validation passed: + +- `cargo test -p pod feature --lib` +- `cargo test -p pod task --lib` +- `cargo test -p pod --lib` +- `cargo test -p llm-worker --lib` +- `cargo check --workspace --all-targets` +- `cargo fmt --check` +- `git diff --check` +- `./tickets.sh doctor` +- `nix build .#yoi --no-link` + +Reviewer-rerun validation passed: + +- `git diff --check develop...HEAD` +- `cargo test -p pod feature --lib` + +## Review status + +External sibling reviewer approved with no blockers and no required non-blockers before merge. + +## Unresolved risks / follow-ups + +The existing `HostAuthorityGrantSet::grant_all(&descriptor.requested_host_authorities)` behavior remains a builtin-only scaffold, not a real external plugin approval resolver. This is unchanged and explicitly outside this ticket's scope. + +## Ready for merge + +Yes. This clears the API naming prerequisite for `ticket-built-in-feature-tools`. + + ---