feat(helper): reconcile installs, route applies - #669
feat(helper): reconcile installs, route applies#669Alex Shabalin (alex-sparus) wants to merge 2 commits into
Conversation
This stack of pull requests is managed by jj-ryu. |
🎨 Storybook previewUpdated for 7de50ac
|
📋 PR Overview
🔬 Coverage
|
d71975f to
aedf770
Compare
db02c9d to
c39e869
Compare
aedf770 to
feff8d4
Compare
c39e869 to
34cf6b7
Compare
feff8d4 to
ad485a9
Compare
34cf6b7 to
ba85203
Compare
ad485a9 to
f1f2cbb
Compare
ba85203 to
bdb2c00
Compare
f1f2cbb to
9522ed4
Compare
bdb2c00 to
527369e
Compare
527369e to
07dfb5b
Compare
07dfb5b to
0f17cea
Compare
| /// force: a main-thread decision would block on the slot while the holder's | ||
| /// pass may be awaiting a main-queue dispatch — a deadlock, where an | ||
| /// observation merely misreports. Every current caller is an async handler. | ||
| pub fn grant<R: Runtime>(app: &AppHandle<R>) -> Reconciled { |
There was a problem hiding this comment.
This conditional logic looks a little odd; maybe the login items should only be opened either before OR after depending on report type instead of both.
There was a problem hiding this comment.
via AI:
At most one of the two fires per click — the second is guarded on !awaiting_approval_already — so it is either-or already, just spelled as two triggers because they answer different questions: the before-open keeps the "Open Settings" button's promise immediately (and even if the run then stops short), while the after-open is the first-time Enable path, where the approval wait only exists once this run registered. Folding them into one post-run check would delay the first case behind the whole run and drop it on a failed one. Good catch that the doc didn't say "at most one" out loud though — fixed.
| } | ||
|
|
||
| /// The table. Total over the stored decision and all four statuses. | ||
| fn route(helper: &HelperUse, status: Result<RegistrationStatus, String>) -> Route { |
There was a problem hiding this comment.
I think I asked this or an analogous question in a previous PR iteration of this feature: It seems like we might not want to treat refusal as activation failure (depending on how the UI presents back to the user).
There was a problem hiding this comment.
You're right. I'll fix via adding refused: bool into ActivateResult and fixing the consumers
| fn commit_to_register<E: Environment>(env: &E) -> Result<Committed, Reconciled> { | ||
| // A register is a mutation like any other. | ||
| env.gate()?; | ||
| match env.preference() { |
There was a problem hiding this comment.
It's a little hard to see that there still can't be a race where the preference changes between here and the actual register, maybe you can confirm?
There was a problem hiding this comment.
via AI:
The window is real (and known) but can't lose the decision or the end state: a decision is written before its own pass (decide), which then blocks on the run slot until this pass ends and carries the decision out against whatever this pass did. A write that slips past the re-read costs at most one registration that the immediately-following pass removes. The re-read exists to make the common case (a Disable during the slow service calls) never register at all. Documented on commit_to_register now.
9522ed4 to
157884e
Compare
0f17cea to
74e888d
Compare
74e888d to
8b6f076
Compare
8b6f076 to
74c824f
Compare
Problem: the GUI meets an installed helper of unknown build that may be running an activation, and must drive it toward the stored decision — adopt, replace, or remove — without ever interrupting a running activation. Every apply must pick between the helper and the administrator-password prompt, and a helper refusal must never silently become a password prompt. One pass is rarely enough: the platform refuses a register for about a second after an unregister, and approval waits on a human. Solution: one idempotent reconciliation pass — a status-by-goal table with displacement gates re-checked before every mutation and a typed commitment token minted only by the register step's final checks — over a ServiceManagement adapter that registers only after the unregister completion confirms the old process is gone. A pass that finds an activation running ends with a WaitingOnActivation report; the convergence loop re-observes it on its waiting cadence, uncounted against the mutation bound. activation_path routes each apply from reconciled service state, totally over decision and status, and an attempted helper exchange never falls through to the prompt. The sync agent stays a fire-and-defer activation client. The helper preference is device-local: settings import cannot carry or reset it.
Problem: a Finder-replace upgrade leaves the old helper failing signature validation (errSecCSStaticCodeChanged), so reconciliation classifies it RootUnverifiable and removes it without asking Status. The Activating deferral is unreachable in the one scenario it exists for: an upgrade during a running activation kills the old helper and the applying client's result reply is lost. Solution: split what validation gates. Every action stays gated; the Status read from a root peer is not. assessed_status hardcodes the request, so no mutating request can take the relaxed path; a parseable answer from an unverifiable root peer becomes UnverifiableAnswered, and an Activating answer buys exactly one thing: WaitingOnActivation(None), postponing the peer's own replacement. Any other answer, and any probe failure, takes the removal path unchanged. The reply authorizes nothing: build-ID claims are ignored, activation details are withheld from the report, and the post-register verify loop treats the variant as a failure.
157884e to
7b637c2
Compare
74c824f to
7de50ac
Compare
2 participants
Summary
Problem: the GUI meets an installed helper of unknown build that may be running an activation, and must drive it toward the stored decision — adopt, replace, or remove — without ever interrupting a running activation. Every apply must pick between the helper and the administrator-password prompt, and a helper refusal must never silently become a password prompt. One pass is rarely enough: the platform refuses a register for about a second after an unregister, and approval waits on a human.
Solution: one idempotent reconciliation pass — a status-by-goal table with displacement gates re-checked before every mutation and a typed commitment token minted only by the register step's final checks — over a ServiceManagement adapter that registers only after the unregister completion confirms the old process is gone. A pass that finds an activation running ends with a
WaitingOnActivationreport; the convergence loop re-observes it on its waiting cadence, uncounted against the mutation bound.activation_pathroutes each apply from reconciled service state, totally over decision and status, and an attempted helper exchange never falls through to the prompt. The sync agent stays a fire-and-defer activation client. The helper preference is device-local: settings import cannot carry or reset it.Stacked on #668.
Test Plan
cargo testgreen: scripted-world reconciliation tests over the full status-by-goal table (adoption, replacement, deferral, verify branches, gate re-checks, mid-run Disable declining the register), the complete activation-routing table (every refusal proven never to reach the prompt), convergence-loop bounds, sync-agent dispatch outcomesDocs
Prior review
Carried over from Scott-approved PRs, byte-identical or nearly:
service.rs(4 lines differ, a test string) from #620/#635,preferences.rs/prefs.rs/settings_io.rsfrom #620.reconcile.rskeeps #635's shape (status-by-goal table, scripted-world tests) but is substantially rewritten — no drain machinery, deferral instead — andactivation_path.rs/helper_permission.rsdiffer by ~20% from the #636-era versions. Review effort belongs on those three.