Skip to content

refactor(tui): re-land FEAT-020 plugin command shapes on main - #5865

Open
aboimpinto wants to merge 10 commits into
Hmbown:mainfrom
aboimpinto:feat/FEAT-020-reland-plugins-main
Open

refactor(tui): re-land FEAT-020 plugin command shapes on main#5865
aboimpinto wants to merge 10 commits into
Hmbown:mainfrom
aboimpinto:feat/FEAT-020-reland-plugins-main

Conversation

@aboimpinto

@aboimpinto aboimpinto commented Sep 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

No-Issue: FEAT-020 is tracked in umbrella #5316, which must remain open for the remaining command-decomposition work.

Summary

Re-lands FEAT-020 on current main. The original implementation PR #5657 was merged into codex/v0912-integration-20260823, not main; neither its merge commit ece99d5 nor its feature head is an ancestor of current main.

This forward-port preserves the original seven authored commits, adds one current-main reconciliation commit, and includes one focused Boy Scout test repair for failures reproduced on current main. It:

  • adds the contract-owned CommandPluginContext facet and typed portable plugin/marketplace outcomes;
  • adds the TUI PluginAdapter, keeping App, registry, config, async/network, marketplace, Kimi, and legacy-tool machinery host-side;
  • moves /plugin to ContextualCommand::from_contract with exact WORKSPACE | PRESENTATION | PLUGIN authority;
  • keeps missing facets fail-closed with Command capability unavailable: <facet>;
  • removes plugins from both migration-frontier representations, leaving [config, core, debug, session];
  • preserves current-main /plugin names, aliases, usage, rendering, trust/install/update/uninstall behavior, marketplace CRUD, and Kimi import behavior.

Current-main reconciliation

This is intentionally not a blind replay of the old integration branch:

  • PLUGIN is appended at 1 << 12; current main has already published PROJECT = 1 << 10 and SKILL_GROUP = 1 << 11 through FEAT-021/022.
  • FEAT-019 memory, FEAT-021 project, and FEAT-022 skills facets/adapters/tests remain intact.
  • Current-main marketplace-backed plugin suggestions are projected through the plugin facet and retain their exact visible output and no-mutation guarantee.
  • The one-shot “Plugins changed on disk” reload nudge remains host-owned and visible through /plugin list.
  • Marketplace add/install uses the current shared document loader and install-resolution machinery also used by the Runtime API.
  • Review tokens use the current canonical LoadedPlugin::review_token implementation.
  • The retired computer-use built-in catalog from the old integration branch is not resurrected; current main does not contain that crate/catalog. The portable state permits an optional future host catalog without inventing one.

Current-main CI blocker repairs

  • Scope symlink-refusal tests to Unix, where they actually create symlinks; this removes the current-main Windows -D warnings failure without weakening Unix coverage.
  • Make the binary plugin acceptance scan all visible trust-command candidates, so a retained partial transcript command cannot hide the complete canonical review token.
  • Replace the stale mobile 0.0.0.0 warning smoke check with explicit rejection and loopback-guidance assertions, matching the runtime's enforced security boundary.

Validation

  • cargo fmt --all -- --check — PASS
  • cargo test -p codewhale-command-contract --lib --locked — 43/43 PASS
  • plugin command tests — 20/20 PASS
  • FEAT-020 public registration/dispatch tests — 3/3 PASS
  • plugin adapter tests — 3/3 PASS
  • current-main marketplace suggestion and one-shot reload-nudge preservation tests — PASS
  • related current-main Runtime API plugin/marketplace tests — 6/6 PASS independently
  • python3 scripts/test_check_command_crate_boundaries.py — 8/8 PASS; live boundary gate PASS
  • python3 scripts/test_check_command_migration_manifest.py — 56/56 PASS; live migration gate PASS
  • Cucumber acceptance — 16/16 PASS
  • CI-equivalent strict workspace Clippy with -D warnings — PASS
  • git diff --check — PASS

A single-process full TUI lib run exercised all tests: 11,536 passed; tests competing for the process-global runtime owner lock failed together, while every plugin/marketplace Runtime API failure relevant to this change passed independently with an isolated runtime directory. The authoritative CI uses one-process-per-test cargo nextest and will remain the merge gate. This PR stays draft until the complete required matrix is green.

Provenance

Paulo Aboim Pinto

Paulo Aboim Pinto added 8 commits September 3, 2026 10:01
Paulo Aboim Pinto
feat(FEAT-020): add plugin capability, plugin facet, and typed outcom…
ccaccec 
…es to command contract

- CommandPluginContext: object-safe synchronous facet covering registry reads/mutations, async-bridged install/update/uninstall with sync receipts (D11), export, legacy scan, kimi managed import, and marketplace
- Portable DTOs: PluginSummary/Detail/Diagnostic/McpServerDetail, mutation outcome+receipt, export receipt, legacy tool+scan, managed candidate+scan, marketplace catalog/candidate/add/state, suggestion
- PLUGIN = 1 << 10 capability bit and one plugin envelope slot with with_plugin builder
- Contract tests: object safety, field/variant closure, sync receipt outcomes, exact-hash mismatch, managed/marketplace portability, envelope transport, duplicate-slot rejection, bit stability
- Contract boundary gate green; workspace compiles; fmt clean; 23/23 contract tests pass

Generated with Claude Code

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
refactor(FEAT-020): registry mutations return Result<(), String> not …
1f79220 
…misleading receipts

Code-review finding: trust/enable/disable/revoke_trust returned a PluginMutationReceipt
with outcome always NoChange, which is semantically wrong (NoChange means 'already up
to date' in the install/update path). The host registry returns Result<(), String>;
the handler renders the action word from its own dispatch arm and re-reads detail
for post-mutation state. Return Result<(), String> — the exact-minimum typed surface.

Generated with Claude Code

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
feat(FEAT-020): add TUI plugin adapter with full host mapping and res…
5af7390 
…tricted exposure

- PluginAdapter implements CommandPluginContext against App: registry reads (summaries/detail/diagnostics/validation/suggest), registry mutations (trust/enable/disable/revoke with skill-cache + active-skill side effects), async-bridged install/update/uninstall with synchronous receipts (D11), export, legacy scan, kimi managed scan/install, marketplace state/add/remove/install (incl. builtin official catalog)
- CommandContextBundle grows to eleven slots with plugin; contexts() exposes plugin only for PLUGIN capability
- Portable conversion helpers: summary/detail/mcp server/diagnostic/marketplace diagnostic/mutation receipt/export receipt/legacy tool/marketplace candidate/catalog
- kimi_import: scan_managed_plugins_portable wrapper; group modules made pub(crate); plugin_network_policy/run_async exposed
- Adapter tests: host-data projection, registry mutation + suggest behavior, restricted exposure (3 tests)
- Full TUI lib suite 11395/0; boundary gate green; fmt clean

Generated with Claude Code

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
feat(FEAT-020): convert plugin group to portable handlers with fake-f…
f967515 
…acet parity

- mod.rs: portable plugins() dispatch consuming workspace/presentation/plugin facets; legacy shell builds bundle and delegates (Phase 6 replaces with from_contract)
- render.rs: render_bundle_detail/escape helpers consume portable PluginDetail + presentation facet
- legacy.rs: consumes PluginLegacyScan; kimi_import.rs: consumes PluginManagedScan; marketplace.rs: consumes PluginMarketplaceState with localized plan text
- Presentation facet: key_to_plugin_message_id maps all 52 plugin keys; source_path carried for marketplace provenance
- Contract: PluginSuggestion.state_label, PluginDetail.inventory_summary, PluginMarketplaceCatalog.source_path, reload() facet method
- Tests: 18 plugin tests converted to the portable shell path; full parity preserved
- Full TUI lib 11394/0; contract 23/23; boundary gates green

Generated with Claude Code

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
feat(FEAT-020): register /plugin through the portable bridge and shri…
7891068 
…nk both frontiers

- PluginsCmd implements contract RegisterCommand<CommandResult> with exact WORKSPACE | PRESENTATION | PLUGIN; PluginsCommands group registers via ContextualCommand::from_contract
- plugins_contextual destructures facets with safe missing-facet errors; transitional App shell now test-only
- Public dispatch tests: exact capability set, undeclared facets absent, public seam dispatch, no-panic matrix (3 tests)
- Remove plugins from PENDING_GROUPS and scripts/command-migration-topology.json frontier (same commit)
- Migration fixture updated for six-group frontier; feat015 legacy-assertion test adds plugin to MIGRATED
- All gates green: contract 23/23, TUI lib 11397/0, migration/boundary/CI fixtures + live gates, fmt, diff hygiene

Generated with Claude Code

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
style(FEAT-020): strict clippy -D warnings clean across the workspace
c5f1746 
- Fix clippy findings in FEAT-020 plugin files: identical if blocks (contract.rs), useless as_ref/map (marketplace.rs), useless format + redundant closure (render.rs), manual unwrap_or_default (mod.rs), collapsible if (contract tests)
- Boy Scout: repair pre-existing lints outside FEAT-020 scope (computer-use linux.rs &PathBuf->&Path, config catalog tests contains()/type_complexity)
- cargo clippy --workspace --all-targets --locked -- -D warnings exits 0 with zero warnings

Generated with Claude Code

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
fix(FEAT-020): route rollback uninstall through the plugin facet (D1)
78e0f33 
rollback_hash_mismatch called crate::plugins::install::uninstall directly
from the portable handler, a TUI-owned executable dependency that violates
the D1 boundary and would break the FEAT-040 physical move. Add
CommandPluginContext::uninstall_path(name, plugins_dir) - a file-level
rollback removal with no registry resolution or skill side effects - and
route the content-hash-mismatch rollback through it. The host adapter owns
the crate::plugins call.

Verified: contract 23/23, plugins group 18/18, plugin-scoped TUI suite
205/0, clippy -D warnings clean.

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
Paulo Aboim Pinto
fix(FEAT-020): reconcile plugin slice with current main
b48fd79 
Append PLUGIN after the capability identities already published by FEAT-021/022 and preserve current-main behavior for marketplace-backed suggestions, one-shot reload nudges, canonical review tokens, and shared marketplace document validation. Do not resurrect the retired computer-use builtin catalog.

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
@aboimpinto aboimpinto mentioned this pull request Sep 3, 2026
Open
60 tasks
Paulo Aboim Pinto
test: repair cross-platform plugin CI blockers
f917101 
Scope symlink refusal tests to Unix, where symlinks are actually created, so Windows all-feature test compilation remains warning-free. Scan all visible trust-command candidates in the binary acceptance harness so a retained partial command cannot hide the complete canonical review token.

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
@aboimpinto

aboimpinto commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

I checked both failed jobs and pushed focused repairs in f917101c8.

  • Windows failed before running tests because three Unix-only symlink-test locals were unused under cfg(not(unix)). The same diagnostics are present on current main (main CI job). The tests are now scoped to Unix, where they actually create and reject symlinks.
  • Ubuntu ran 14,234 tests and had one failure: the binary plugin acceptance timed out while extracting the canonical /plugin trust review token. The token was visibly rendered, but an earlier partial command in the transcript caused the helper to reject the first candidate without scanning for the complete one. Current main showed the identical failure on macOS (main CI job). The helper now scans all candidates, with regression coverage.

Local post-fix checks pass:

  • 2/2 symlink refusal tests
  • review-token parser regression test
  • complete binary plugin lifecycle/Skill/stdio-MCP acceptance test
  • formatting and diff hygiene

A fresh full matrix is now running. The PR remains Draft.

Paulo Aboim Pinto

Paulo Aboim Pinto
test: align mobile smoke with loopback enforcement
69a1734 
Replace the stale 0.0.0.0 warning expectation with an explicit non-loopback rejection check and verify that the error explains the enforced loopback-only boundary.

Signed-off-by: Paulo Aboim Pinto <paulo.aboim.pinto@gmail.com>
@aboimpinto

aboimpinto commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

The new matrix fixed the previous Ubuntu and Windows failures: both full test jobs are green.

The remaining Mobile runtime smoke failure was another stale current-main expectation: the script still expected an insecure mobile server to default to 0.0.0.0 and print a warning, while the runtime now correctly enforces loopback-only mobile binding. Current main fails the same job.

I pushed 69a17340c, replacing that obsolete warning check with two security assertions:

  • --mobile --host 0.0.0.0 must be rejected;
  • the rejection must explain the loopback-only boundary.

The complete smoke script now passes locally, 9/9, against the current debug binary. Shell syntax and diff hygiene also pass. A fresh matrix is starting, and the PR remains Draft.

Paulo Aboim Pinto

@aboimpinto
aboimpinto marked this pull request as ready for review September 3, 2026 10:50
@aboimpinto
aboimpinto requested a review from Hmbown as a code owner September 3, 2026 10:50
@aboimpinto

aboimpinto commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

The fresh head matrix for 69a17340c is fully green.

Passed checks include Linux, macOS, Windows, lint, Safety gate, Mobile runtime smoke, Runtime integrations, OHOS cargo check, npm wrapper smoke, signed-off-by, link, version drift, and GitGuardian. The remaining skipped jobs are intentional workflow outcomes.

The previously failing jobs are confirmed repaired:

  • Ubuntu test: PASS
  • Windows test: PASS
  • Mobile runtime smoke: PASS (9/9 assertions)
  • macOS test: PASS

The branch is clean and GitHub reports it mergeable. I have marked PR #5865 Ready for Review.

Paulo Aboim Pinto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Hmbown Hmbown Awaiting requested review from Hmbown Hmbown is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@aboimpinto