feat(skills): FEAT-022 skills command shapes and retained-host validation - #5829
Merged
Conversation
…slot to command contract - CommandSkillGroupContext trait with exact-minimum typed delegates (D1) - Portable values: SkillRegistryProjection, SkillEntry, mutation receipts, remote registry/suggest/sync outcomes, activation/review outcomes, snapshots - skill_group envelope slot (eleven total) + with_skill_group builder (D4) - Shared CommandSkillsContext unchanged (D2 regression test) - 13 new contract tests: object-safety, variant closure, slot behavior Generated with Claude Code
…osure - SkillGroupAdapter implements CommandSkillGroupContext: discovery projection, activation with plugin-authority verification + side effects, synchronous mutation receipts, remote registry/suggest/sync, review, snapshots, approval - Bundle gains the eleventh adapter slot; 'ten facets' comment updated (D3) - Contract refinements locked during implementation: SkillSyncOutcome policy variants; ReviewOutcome::Ready payload dropped (side effects host-side) - 12 adapter tests + 2 mapping unit tests; TUI lib suite green Generated with Claude Code
- /skills and /skill handlers consume CommandSkillGroupContext (+ shared SKILLS for /skill per D4): discovery projection, remote/suggest/sync, receipts, activation, safe missing-facet errors; byte-identical output - Cache-refresh policy moved to the portable layer via SKILLS.refresh_skill_cache (D2); /skill new alias is handler-side parsing; adapter delegates simplified - Dead TUI BundledSkillTier::heading removed (its only consumer was the migrated listing; heading now lives on the portable SkillBundledTier) - Transitional shell execute() delegates to contextual dispatch (Phase 6 replaces it with the contract bridge); run_skill_by_name + host machinery retained co-located for FEAT-042 - 25 fake-facet parity tests Generated with Claude Code
…atch - /review destructures group + SKILLS (D4); host performs discovery + side effects via run_review; handler renders exact not-found error and SendMessage - /restore destructures group only (no MODE_POLICY, D4); host owns SnapshotRepo + approval state; handler owns parsing, trust gate, and exact listing text - Transitional shell execute() delegates to contextual dispatch (Phase 6) - 12 fake-facet parity tests (exact strings, trust gate, host-error boundary) Generated with Claude Code
…ink both frontiers - skills/mod.rs registers all four commands via ContextualCommand::from_contract; legacy FunctionCommand registrations removed (metadata now contract CommandInfo with description_key; transitional shells removed) - skills removed from PENDING_GROUPS and topology frontier in the same change (D8); MIGRATED_GROUPS gains skills/skill/review/restore - Migration gate extended with RETAINED_HOST_MACHINERY (fail-closed FEAT-042 tracking for run_skill_by_name/activate_skill_with_task/activate_skill that keep &mut App by design); fixtures updated - 3 public-dispatch/registration tests; all gates green (migration fixtures + live, boundary fixtures + live, CI wiring, locale parity) Generated with Claude Code
…tion for empty-handler groups - 2 hermetic fixture tests: retained-host exemption and signature-loss detection - retained-host validation now runs before the empty-handlers early exit so a group whose retained machinery all lost its App signature still fails closed Generated with Claude Code
- feat022_aliases_dispatch_through_public_seam: jinengliebiao / jineng / shencha resolve through the registry to the portable handlers - Collapse nested if in /skills inspect path (clippy collapsible_if) - Boy Scout: allow(clippy::too_many_arguments) on two pre-existing baseline signatures (runtime_threads::accumulate_runtime_cost_coverage, underwater::render_launch_composer); strict clippy gate was RED at the pristine baseline f76252d on this toolchain - Full gate set green: clippy -D warnings workspace-wide, TUI --tests 277/277, contract 29/29, all Python gates Generated with Claude Code
Remove the synthetic SKILLS dependency and cache refresh from /review because the baseline performs neither. Keep /skill mutation refresh behavior unchanged and add exact call-count coverage for install, update, uninstall, and trust. Verified with focused contract/TUI tests, public dispatch tests, migration gates, the full default Cucumber suite, and targeted clippy -D warnings.
Resolves the underwater.rs overlap: the 0.9.12 launch card rewrote the launch composer (adding the bottom-rule parameter), which supersedes the clippy-allow hunk this branch carried. Import placement follows the 0.9.12 file.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6c5b6219-3dc5-40c0-a8c1-9246ebaaf154) |
Contributor
|
Claude finished @Hmbown's task in 3m 11s —— View job Claude is reviewing this PR
|
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces a wide new contract facet and rewires multiple commands across the contract/TUI boundary (including network- and snapshot-related behavior), which warrants final human verification despite the added tests and gates.
Pull request overview
This PR implements FEAT-022 by migrating the TUI “skills” command group (/skills, /skill, /review, /restore) onto the portable command-contract “contextual dispatch” pathway, while adding an explicit retained-host validation mechanism to keep certain &mut App dispatcher helpers fail-closed until FEAT-042 extracts them.
Changes:
- Adds a new
CommandSkillGroupContextfacet (plus portable DTOs) to the command-contract, and wires a correspondingskill_groupslot intoCommandContexts. - Introduces a TUI-side
SkillGroupAdapterthat performs all concrete host work (skills discovery/mutation, plugin authority checks, network policy, async bridging, snapshot list/restore) while handlers remain contract-typed and capability-scoped. - Extends the migration gate (
check-command-migration-manifest.py) withRETAINED_HOST_MACHINERYtracking and validation, and updates topology/frontier fixtures/tests to reflect the now-migratedskillsgroup.
File summaries
| File | Description |
|---|---|
| scripts/test_check_command_migration_manifest.py | Updates frontier expectations and adds hermetic tests for retained-host exemptions and fail-closed validation. |
| scripts/command-migration-topology.json | Removes skills from the pending frontier list. |
| scripts/check-command-migration-manifest.py | Adds retained-host machinery declarations and validation logic, plus selector matching support. |
| crates/tui/src/tui/underwater.rs | Adds a targeted clippy allow for a pre-existing too-many-arguments signature. |
| crates/tui/src/skills/system.rs | Removes the TUI-side tier heading helper now represented in the contract tier enum. |
| crates/tui/src/runtime_threads.rs | Adds a targeted clippy allow for a pre-existing too-many-arguments signature. |
| crates/tui/src/commands/mod.rs | Adds FEAT-022 registration/dispatch tests ensuring the four commands use the public portable seam. |
| crates/tui/src/commands/groups/skills/skills.rs | Ports /skills and /skill to contextual dispatch; keeps specific App-carrying dispatcher helpers co-located for FEAT-042. |
| crates/tui/src/commands/groups/skills/review.rs | Ports /review to contextual dispatch using only the skill_group facet and preserves baseline behavior. |
| crates/tui/src/commands/groups/skills/restore.rs | Ports /restore to contextual dispatch with host-owned snapshot operations and a handler-owned approval gate. |
| crates/tui/src/commands/groups/skills/mod.rs | Registers the four commands via ContextualCommand::from_contract instead of legacy FunctionCommand. |
| crates/tui/src/commands/contract.rs | Adds SkillGroupAdapter, maps host types to portable DTOs, and exposes the new facet in the command envelope. |
| crates/command-contract/src/tests.rs | Adds contract-level tests for the new facet, DTO semantics, and envelope slot behavior. |
| crates/command-contract/src/handler.rs | Adds the skill_group slot to CommandContexts and wiring in ContextParts. |
| crates/command-contract/src/facets.rs | Defines CommandSkillGroupContext and all FEAT-022 portable skill/snapshot/approval types. |
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Hmbown
enabled auto-merge (squash)
September 2, 2026 17:01
60 tasks
Garfield1985
pushed a commit
to Garfield1985/DeepSeek-TUI
that referenced
this pull request
Sep 5, 2026
…umber hang, mobile smoke, release receipts) - Windows warnings (§5.1): restructure link and error bindings in document.rs and runtime_api/tests.rs so variables exist only on targets that use them. Note: Windows CI cannot be run locally on this host. - macOS PTY acceptance (§5.2): in plugin_e2e_acceptance, iterate over all marker match indices in review_confirmation_in_text rather than only the first, preventing false stalls on the composer's typed command echo. Full 31/31 cucumber tests passed. - Mobile smoke (§5.3): update scripts/mobile-smoke.sh Test Group 3 to assert loopback security warning, matching the loopback-only hardening from e80be8c. 9/9 passed. - Release-note receipts (§5.4): cite genuine issues (Hmbown#5533, Hmbown#5831, Hmbown#5825, Hmbown#5829, Hmbown#5801, Hmbown#5815, Hmbown#5826, Hmbown#5286, Hmbown#5809, Hmbown#5778, Hmbown#5725, Hmbown#5701, Hmbown#5712, Hmbown#5747, Hmbown#5782, Hmbown#4394) in CHANGELOG.md; filter commit-body scrape noise (#000, Hmbown#1, Hmbown#24, Hmbown#26, Hmbown#1834, #142352) in check-feature-release-notes.sh. check-versions passes in strict release mode (71 receipts checked). Evidence: - cargo nextest run --workspace --all-features --locked: 14229 passed; 0 failed; 15 skipped - cargo clippy --workspace --all-targets --all-features --locked -- -D warnings: clean (0 warnings) - cargo test --workspace --all-features --doc --locked: 3 passed; 0 failed; 8 ignored - cargo test -p codewhale-tui --test cucumber --features long-running-tests --locked: 31 passed; 0 failed; 0 ignored - scripts/mobile-smoke.sh: 9 passed; 0 failed - scripts/release/check-versions.sh --require-dated-release: Feature release-note receipts OK (71 linked issues); Version state OK - Python policy gates: check-provider-registry, check-command-crate-boundaries, check-dead-code-budget, check-runtime-contract-budget, check-readme-translations, check-tui-locale-parity all PASS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #5825
Harvested from PR #5825 by @aboimpinto
FEAT-022: skills command shapes and retained-host validation.
Co-authored-by: Paulo Aboim Pinto paulo.aboim.pinto@gmail.com