Repository navigation
[CTX-1313] fix(plugin): keep fresh CLI installs disabled (#2204) - #2212
Conversation
Closes #2204 Keep local, git, and archive CLI installs staged until explicit enable while preserving existing update desired state and package security gates.
Extend the real CLI regression coverage for enabled and disabled updates, capability rejection rollback, and no-package-execution staging evidence.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 51 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Xuepoo
left a comment
There was a problem hiding this comment.
Independent review — separate agent (ctx-1313-reviewer, provider custom), HEAD d23def8
Verdict: LGTM (findings: none blocking; three info-level observations, no defect).
Checks run by the reviewer on the exact PR tree:
git diff --name-status origin/main d23def8f→M crates/bitty-terminal/src/plugin.rs,M crates/bitty-terminal/tests/cli_plugin.rs(exactly two files).cargo +1.85 check -p bitty-terminal --offline→Finished dev profile ... in 20.26s(MSRV 1.85 clean; noif ... && letlet-chains, so no E0658).cargo nextest run -p bitty-terminal --test cli_pluginon d23def8 →18 tests run: 18 passed, 0 skipped, includingplugin_fresh_zero_capability_install_is_staged_until_explicit_enableandplugin_external_install_load_run_update_remove_live.
Acceptance mapping (#2204):
- Fresh CLI installs remain disabled:
install_local_pathnow buildsLocalInstallOptions::staged_and_disabled()instead of::default();install_git_sourceandinstall_archive_sourceboth funnel through it, so local/Git/archive fresh installs are all staged disabled. - Explicit enable permits activation; update preserves enabled state; capability rejection leaves index/grants unchanged — each covered by a named assertion in the new tests.
Residual risk (reviewer): LocalInstallOptions::default() stays enabled for library/embedder callers — intentional scoping (only the CLI path is changed, which is what #2204 asked); just ci-local was not run by reviewer or implementer (per lane rules only one full local gate at a time); remote CI is the remaining signal.
Head note: verification was performed against d23def8 specifically (the shared checkout HEAD was older).
Recorded by the guard tick; this is a reviewer-of-record comment because the environment shares a single GitHub identity (gh pr review --approve on an own-PR is refused).
|
Historical disposition for PX-7123/PX-7124 and CTX-1326: source remains APPROVE for merged commit D1/D2 remain historical delivery disclosures: the external merge and CTX-1313 completion preceded mandatory real local Quality acceptance; the current captured records contain no non-author D3 corrections: enabled-v2 evidence is state persistence/retained-v1-directory evidence, not v2 activation, grants, or version proof; v1/v3 activation evidence is separate. Malformed-Lua negative evidence is staging plus empty |
Priority: P0 | Area: plugin | Labels: fix,P0,area:plugin | Milestone: 0.0.24 | RFC: P0-AC-027 through P0-AC-030 | Task: CTX-1313
Follow-up correction task: CTX-1326 is P1 release metadata and delivery-evidence correction work; the merged source task remains the original P0.
Summary
Fresh CLI plugin installs use the staged disabled policy until an explicit
bitty plugin enable <id>. Existing updates preserve the plugin's currentenabled/disabled state. The public library default
(
LocalInstallOptions::default()) is unchanged, so existing runtime/developercallers are unaffected — only the CLI install path opts into staging.
The local path and the successful tails of Git/archive acquisition converge on
this policy. Captured validation does not include a successful Git transport and
installation run; Git/archive convergence is source-path evidence only.
Install has no VM entry in the install path. The malformed-Lua staging test
observed a successful disabled install with empty
discover()output. This isbounded negative evidence; it is not an instrumented invocation counter or full
hostile-hook proof. Consent denial and stdin EOF fail closed before the package
is committed.
Closes #2204.
Changes
crates/bitty-terminal/src/plugin.rs(+9/-4)LocalInstallOptions::staged_and_disabled().report_installno longer appends “enabled” to the success line.updates preserve existing state.
crates/bitty-terminal/tests/cli_plugin.rs(+245/-16) — real-binary (no mocks) CLIregression coverage:
discovered_idsare empty while staged, then activate only after enable.it does not activate v2 or explicitly compare v2 grants/version fields. The
separately exercised v1 and v3 paths provide the actual activation evidence;
disabled update stays disabled with empty discovery until enable.
current.json)bytes and parsed records/grants before and after, and assert no committed v2
directory. They do not hash every retained package file.
discover()is empty. This testdoes not assert
discovered_ids(), use an instrumented invocation counter, or provethe full hostile-hook surface.
Fixes a reviewed blocker
Independent review (PX-6926/PX-6927/PX-6928/PX-6931) raised NEEDS-FIX on the first commit:
the enabled-update execution proof had been replaced and full index/grant equality was
missing. The repair commit adds the scoped state evidence; cumulative independent review
of the two-file source delta returned APPROVE / SOURCEAPPROVE (PX-6955, PX-6991).
Evidence
Rebased onto
origin/mainat4d1b2596(dropping the superseded[CTX-1317]prerequisite, which already landed on
mainvia #2209); delta is exactly two files.just fmt-checkjust clippy(workspace--all-targets+bitty-terminaldev-tools)cargo nextest run --workspace --lockedcargo nextest run -p bitty-terminal --test cli_plugin --lockedThe preserved independent review also records stable
just checkand explicitjust testsuccess, plus native typecheck and the Windows GNU cross-check. Themandatory real local Quality result remains incomplete/pending in the preserved
capture; the shard results are not presented as a full Quality pass. Successful
Git transport/install was not run in the captured evidence, and the existing
archive happy path does not prove the disabled bit.
Historical delivery disclosure (CTX-1326 / PX-7123 / PX-7124)
2026-10-17T19:57:09Z, and CTX-1313 wascompleted at
2026-10-17T19:57:39.794888409Z, before mandatory real localQuality acceptance was recorded. At the earlier review capture, the Quality attempt had failed during action
preparation and its recovery capture was incomplete. The stable validator
subsequently ended at
2026-10-17T20:20:24.912174024Z. The separate finaladdendum reviewed at
2026-10-17T20:25:14Z, recorded in PX-7137/PX-7138,accepts SOURCE + FINAL INSPECTED GATE EVIDENCE: full check, explicit test,
native typecheck, Windows GNU cross-check, and real local Quality all PASS.
The original incomplete capture is retained unchanged. This later acceptance
does not establish compliance before publication or merge and does not
resolve historical D1/D2 or accept release readiness.
APPROVEDreview from a non-author;the recorded PR review is
COMMENTEDby the shared PR-author account. Theexact required
Quality gatescontext is absent whileQuality gates (1)andQuality gates (2)shards passed. The historical merge/protection mechanism isunestablished by these records; this disclosure does not assert a bypass actor
or mechanism.
CodeRabbit inline comment was captured.
2026-10-17T20:38:08.650360041Z, with commander acceptance PX-0430 and PR [CTX-0146] fix(vt): answer DA/XTGETTCAP/DECRQM queries #240merged as
61a4981d4898f87c8995ac9b310a1de030e16be6. Overall D4 remainspending because the separately owned plugin package-guide synchronization
handoff is unfinished. This documentation completion does not establish
complete security acceptance or release readiness.
R-004 / Issue #2206 and S1 remain blocked and separately owned. Fresh owner release
approval remains required before any tag, release, or publication.