refactor: tighten session ownership and consolidate runtime services - #618
Draft
pgherveou wants to merge 18 commits into
Draft
refactor: tighten session ownership and consolidate runtime services#618pgherveou wants to merge 18 commits into
pgherveou wants to merge 18 commits into
Conversation
Account, device, core, product, and settings stores used divergent writers; replacement could weaken secret-file permissions. Route them through NamedTempFile with private creation, file synchronization, atomic replacement, and Unix parent-directory synchronization. Preserve existing store formats. Regression tests cover initial/replacement permissions and cleanup when replacement fails.
Remove the CLI WebSocket implementation and expose a direct-node connection API in truapi-provider. Preserve CLI chain routing, the People sentinel, and optional E2E route widening. The provider owns shutdown and response-stream lifetime; a take-once response contract and loopback test cover buffered delivery, second-stream termination, close, and late send.
Concurrent state transitions previously delivered callbacks out of order. Queue snapshots while holding the state mutex and let one dispatcher drain outside it, preserving reentrant callback support. Initial announcements follow the same queue. Regression tests coordinate concurrent callbacks and a callback that re-enters the runtime.
An ordinary dropped request now removes its pending registration. Subscription setup retains a bounded tombstone until a late acknowledgement supplies its upstream ID; the acknowledgement owns best-effort unsubscribe cleanup across cancellation races. Active and pre-activation queues are bounded and overflow terminates with an observable error. Numeric subscription IDs retain their JSON type. Tests exercise cancelled silent requests, acknowledgement races, queue limits, and terminal-error delivery.
Carry a slots_exhausted flag beside diagnostic text in renewal failures, including the UniFFI enum. Local CLI policy downcasts its typed failure through anyhow context instead of parsing wording. External-wallet compatibility classification remains at that boundary. Regenerate Swift serialization and test classification with unrelated diagnostic wording.
Store local identity, activation generation, entropy, and grants together. Validate the selected AuthoritySession while copying its zeroizing entropy so concurrent activation cannot substitute another wallet. Revalidate after approval and metadata/ring suspension points. Already-admitted synchronous work may finish with its owned secret. Tests pause approval across logout/replacement and verify stale tokens cannot reacquire entropy.
Install a Rust session-storage adapter before background restore starts. Session activation now prepares durable CLI state and swaps paths/maps together before exposing Connected; failures disconnect while preserving the previous mounted namespace. AuthPresenter becomes observational. Persistence updates stage prospective maps before changing memory. Tests inject directory, core-file, and current-user pointer failures. This is coherent namespace publication, not a multi-file filesystem transaction.
Move event formatting, span tracking, level parsing, and reload initialization into the leaf truapi-logging crate. Each artifact retains its static Logger, prefix, and output sink. OnceLock records failed installation when an embedding application already owns the subscriber, avoiding replacement. Tests cover formatting, span updates, level parsing, and foreign-subscriber preservation.
Move statement, Bulletin, and smart-contract allocation out of the SSO responder into signing_host::allowances. Local calls, renewal, and SSO now use the same workflows with an explicit selected AuthoritySession. Derive candidate/allowance secrets from that session and revalidate around suspension points before submission or sensitive return. Keep transport response mapping at SSO boundaries and native/WASM implementation differences explicit.
Move permission prompting/storage, device-key persistence, feature discovery, and dotNS traversal to application services. Keep stored permission decisions, codecs, derivation, and ABI policy in host_logic. Update Rust consumers and integration imports, and document dependency direction. Public Rust orchestration paths change; wire contracts and platform callback signatures do not.
Use named operations for caller-owned account normalization, session acquisition, and signing confirmation across current and legacy entrypoints. Keep request-specific permission ordering and error conversion visible at their boundaries. Pairing and signing hosts reuse one ring-key ownership validator while retaining validation at both trust boundaries. Capability modules already moved in the mechanical PR now contain only these policy changes.
Share storage projections and a block/collection/ring-bound snapshot source between Subxt and raw RPC. Require status, contiguous nonempty pages, and enough members for the included prefix. Replace raw byte-offset status decoding and missing-status promotion with metadata decoding. Raw readers now require Metadata. Tests cover gaps, duplicate/empty pages, missing status, incomplete prefixes, and metadata projections.
Introduce a validated protocol model between rustdoc extraction and emitters. Resolve frame IDs, names, sensitivity, host initiation, referenced types, and shared wrapper versions once; reject collisions before version filtering. Emitters query the resolved model directly. Existing API outputs retain schema hash 0449982638d57658; an acronym fixture now uses consistent HTTP_SERVER spelling. Golden and idempotence tests cover Rust, TypeScript, and platform bridges.
Remove obsolete normalization shims, redundant service arguments and borrows, and position the shared ownership helper before tests. These follow-ups preserve the previous commits' behavior and keep the full native/WASM Clippy checks clean.
A late acknowledgement correctly triggered unsubscribe, but notifications received before that acknowledgement remained in the unclaimed buffer. Repeated cancelled setups could therefore exhaust the buffer limit and terminate an otherwise healthy connection. Remove buffered items and the active sink together using the same lock order as activation and delivery. Extend the late-acknowledgement regression to deliver an early numeric-ID notification and verify both the unsubscribe request and complete local cleanup. The full workspace all-features suite passes.
The ordered dispatcher used a boolean owner that could remain set forever after a host callback panicked. Introduce a delivery guard that releases ownership on unwind; normal completion disarms it while still holding the queue lock, so it cannot reset a newer dispatcher owner. Callbacks remain outside the state mutex, and their panic still propagates. A regression catches the host panic, performs the next transition, and verifies delivery resumes without poisoning the state lock. Existing concurrent and reentrant delivery tests still pass.
Second-pass review found that an established encrypted SSO channel could outlive its wallet and let later messages select the newly active local session. Capture the AuthoritySession during handshake/resume and carry it through every SSO signing, subtree, ring, and allocation helper instead of reacquiring the current wallet per message. Race the serve operation against explicit local-generation invalidation, including same-wallet reactivation and grant revocation. Public connection-status changes alone cannot detect an unchanged wallet snapshot. Reuse the existing change notifier under the local-session owner, cancel suspended work on invalidation, and revalidate handshake publication/completion. Regression tests verify that an old channel cannot select another wallet or allocate its AutoSigning secrets, and that pending work is cancelled on logout and same-wallet reactivation. The full workspace all-features suite and native Clippy with warnings denied pass.
The local signing and paired CLI batteries both reached all 69 cases but failed the legacy raw-signing example before making a signing call: it expected getLegacyAccounts to enumerate accounts, while the runtime deliberately returns an empty list. Use the caller-owned slot-zero account explicitly, matching the neighboring legacy payload/transaction examples. Regenerate the client and example manifest so the real battery exercises signRawWithLegacyAccount. Keep the runtime policy and the existing Coin Payment/Payment unsupported baseline unchanged.
pgherveou
force-pushed
the
refactor/rust-review-behavior
branch
from
September 7, 2026 09:14
34c31ca to
b87b39b
Compare
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
1 participant
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.
Bind signing and paired SSO work to the wallet generation that authorized it, publish connected sessions only after storage activation succeeds, and give RPC requests/subscriptions explicit cancellation cleanup. Consolidate the duplicated allowance, signing, ring, transport, logging, and protocol-codegen implementations around those ownership boundaries.
Stacked on #617: this PR contains the behavioral and architectural half of the Rust maintainability review. The base branch contains the six mechanical commits; the 18 commits here each describe the problem, resulting behavior, and relevant validation. Review the commits in order.
Changes:
0449982638d57658.Second correctness pass:
Validation:
cargo test --workspace --all-features --locked --offline: 1,287 passed, 20 ignored.cargo +nightly clippy --workspace --all-targets --all-features --locked --offline -- -D warnings: passed.--no-default-features --features 'js smoldot networks' --all-targets: passed.Live CLI end-to-end validation:
Built
target/debug/truapi-hostlocally and regenerated the battery/client withscripts/codegen.sh. Ran the following sequentially against the test network with isolatedXDG_STATE_HOMEstate:scripts/battery.sh --signing-hostscripts/battery.sh --pairing-hostBoth runs exercised all 69 cases. Pairing used a fresh pairing store and a second locally built signing-host process to complete the real SSO handshake and serve remote requests. Coverage includes current/legacy raw and payload signing, transactions, VRF and ring registry/proofs, AutoSigning, permissions, resources, chains, and preimages. The 13 known unsupported cases are the existing Coin Payment/Payment baseline; the baseline was not expanded. Default user wallet state was left unchanged.
Validated source:
34c31caa. Local CLI SHA-256:0ba792d87291d5ea91c10b0ad7000898696cd8320507bd3e28ad7a5eea0af185. Full logs and generated reports remain in the review worktree's ignoredtarget/review-e2eandtarget/batterydirectories.Compatibility and review notes:
slots_exhaustedfield across UniFFI; the Swift binding is regenerated. This changes the binding shape without changing protocol frame IDs or payload layouts.The branch includes #617's reviewed documentation, optional-secret Debug redaction, and explicit capability imports. Validation at
b87b39b0: 851 server tests pass with all features; workspace Clippy (all targets/features), WASM server Clippy, and nightly formatting pass. The live CLI results above were recorded at34c31caa, before these mechanical review corrections; the capability implementation bodies are unchanged by the rebase.