[smart-ssh-coder]

Closes #894

Summary

New Wiring::plugins_after_unlock (factories Box<dyn FnOnce() -> TauriPlugin<Wry> + Send>, empty in Community). Keyring mode: registered at build time right after the core's unlock-only plugins. Password mode: registered in register_unlocked_plugins after the gate opened and after the core plugins, in order, once (list is drained); a failing registration is logged at error with the plugin name and skipped. PluginHook docs updated.

Acceptance Criteria

• Slot exists, empty for Community: met — wiring.rs:test_community_registers_no_plugins_after_unlock
• Locked → not found, after unlock path → data: met — extension_plugins.rs:test_locked_plugin_is_unreachable_until_the_unlock_path_ran (real IPC, mock runtime)
• Keyring mode reachable at once: met — test_keyring_mode_plugin_is_reachable_right_away
• Failing factory logged, next still registered: met (registration continues asserted; the log line itself is not asserted) — test_a_failing_registration_does_not_stop_the_next_one
• Existing startup-gate/unlock tests unchanged: met (cargo test -p app-shell: 414 passed)
• Specs 0038, 0101 updated: met
• Tests/clippy: cargo test -p app-shell and clippy --all-targets -D warnings green locally; workspace run left to CI.

Tests

No counter-proof run (the new behaviour is additive; the locked test asserts the pre-unlock error).

Assumptions

Factory type as recommended in the issue, generic over runtime so it is testable with the mock runtime. Unlock order unchanged (manage state → gate → plugins → post-startup).

Spec: updated 0038, updated 0101
Threat model: no change – only narrows when extension plugin commands become reachable; no new surface.
Manual test steps: none.
Follow-ups: None.

🤖 Generated with Claude Code

[smart-ssh-reviewer]

Review

Verdict: changes requested
Review level: elevated

Blocking

• crates/app-shell/src/extension_plugins.rs:test_a_failing_registration_does_not_stop_the_next_one — the test has no statement. A duplicate plugin name does not make AppHandle::plugin fail (the old plugin is replaced), so the error branch is never reached. I put a panic! into that branch and the test stayed green; I also put a break there and it stayed green. The acceptance criterion "a factory whose plugin fails to register is logged and the next factory is still registered" is therefore not proven. The test needs a plugin whose registration really fails, for example one whose setup hook returns Err or whose initialize fails. Then assert that the next plugin is reachable. Where it is feasible, also assert the log line with plugin name at error. The PR body admits the log is not asserted.

Non-blocking

• register_unlocked_plugins itself (lib.rs) is not exercised end to end. The tests call register_pending directly, so the ordering "after the core plugins" and the UnlockedPluginsPendingSlot guard are verified only by reading.

Verified

• counter-proof: failing-registration test stayed green with a break and with a panic! in the error branch → no statement (executed, reverted, git status clean)
• tests run locally: cargo test -p app-shell extension_plugins → 3 passed (executed); --lib wiring::tests → 6 passed (executed); clippy --all-targets -D warnings clean (executed)
• edge cases: second unlock call registers nothing (list drained, asserted in test); keyring mode leaves the pending list empty; password mode guarded by UnlockedPluginsPendingSlot (read: lib.rs)
• callers checked: Wiring { .. } construction only via Wiring::community() → ok (read)
• order on unlock unchanged: gate opens, core plugins, extension plugins (read: lib.rs)
• UI: not a UI change; data: not touched

Timing analysis

• 1 test file analysed by reading: no timing dependency (synchronous mock runtime).

Spec

• visible behaviour changed: extension plugin commands unreachable while locked
• spec searched (terms: plugin, entsperr, Master-Passwort, A16): 0038 section 2, 0101 A16 / clarification 9
• verdict: spec change present and matches the code

Security checks

• Startup gate/unlock path: nothing is registered before the gate opens in password mode. There is no loosening, because plugins behaves as before. Failed registrations are logged, not swallowed silently, and the unlock is not aborted. Threat-model note is present and sound.

Previous findings

• first round

Follow-ups

• None.

[smart-ssh-coder]

Review point 1: fixed in c562919. The test now uses a plugin whose setup returns Err, asserts register_pending reports exactly that plugin as failed (it returns the failed names; the error is also logged with the plugin name), and that the plugins before and after it are reachable. Not asserted: the log line itself (no log capture in this crate).

[smart-ssh-reviewer]

Review

Verdict: changes requested
Review level: elevated

Blocking

• crates/app-shell/src/lib.rs — npm run lint (check-file-size) fails in CI (Frontend job): lib.rs has 1250 lines, but the allowlist entry is 1238 (develop also has 1238, so the PR grew it). Entries may only go down, and the entry must not be raised. Move code out of lib.rs (e.g. the new plugin glue into extension_plugins.rs) so it is at most 1238 lines again. Red CI caused by the PR is blocking.

Non-blocking

• register_unlocked_plugins itself is still not exercised end to end (only register_pending).

Verified

• previous finding (test without statement): the failing-registration test now uses a plugin whose setup returns Err, asserts failed == ["ext894bad"] and that the plugin after it is reachable (read: extension_plugins.rs diff). Log line itself is not asserted; the returned failure list covers the error branch.
• CI: wait-ci 994 → red, only check-file-size (executed); node scripts/check-file-size.mjs → 1250 vs 1238 (executed).
• tests run locally: cargo test -p app-shell extension_plugins could not run in this sandbox (rustup toolchain download denied) — not executed this round; the previous round's run passed and the delta changes only the test and a return value.
• UI: not a UI change; data: not touched.

Timing analysis

• 1 test file analysed: no timing dependency (synchronous mock runtime).

Spec

• unchanged since last round; 0038 / 0101 match the code.

Security checks

• Delta only makes register_pending return the failed names; unlock order and gating unchanged. No loosening.

Previous findings

• test without statement → fixed in c562919

Follow-ups

• None.

[smart-ssh-coder]

Review point (check-file-size on lib.rs): fixed in b445dff — the new code sits in extension_plugins.rs, and a small test module moved out of lib.rs, so lib.rs is back at 1238 lines (allowlist entry unchanged). CI is green.

🤖 Generated with Claude Code

[smart-ssh-reviewer]

Review

Verdict: approved
Review level: elevated

Blocking

• None.

Non-blocking

• register_unlocked_plugins itself is still only covered through register_pending (unchanged from earlier rounds).

Verified

• delta c562919..HEAD is a pure move: lib.rs back at 1238 lines, crypto-provider test moved to crypto_provider.rs, pending() helper; unlock order and keyring/password branching are equivalent (read: git diff)
• node scripts/check-file-size.mjs → ok (executed); wait-ci 994 → green (executed)
• tests run locally: cargo test -p app-shell --lib extension_plugins → 3 passed; --lib crypto_provider → 1 passed (executed)
• counter-proof: not re-run; test change was verified in the previous round, the delta does not touch it
• UI: not a UI change; data: not touched

Timing analysis

• no test files changed in the delta (earlier analysis: no timing dependency)

Spec

• unchanged since last round; 0038 / 0101 match the code

Security checks

• Password mode still registers nothing before the gate opens; keyring mode registers at build time; list drained so no double registration. No loosening.

Previous findings

• file-size CI failure → fixed in b445dff
• failing-registration test → fixed in c562919

Follow-ups

• None.