You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
- docs/scoping-shared-sessions.md: the design record, as built
(decisions, formats, the canister, the sync engine, the shared OAuth
flow, walkthroughs, security analysis, failure matrix, testing,
release and open items).
- docs/shared-sessions-operations.md: the operator runbook (creating
the canister, keys and rotations, configuration, enabling and
disabling, incidents and disaster recovery, monitoring).
- README.md: a "Shared sessions" section, and the passages that
assumed per-process sessions.
- CONTRIBUTING.md: building and testing the new crates and the
canister, the PocketIC suites, the codec and interface evolution
rules, and releasing five crates (including the one-time placeholder
publish of imcp2-session-api).
- deploy/native/README.md: the shared-sessions drop-in and credentials.
- The privacy policy and directory submission drafts, marked as
needing legal and product sign-off.
The design record and the runbook now describe:
- the sealed envelope's minimum length;
- the corrected touches_effective boundary;
- the shared reporting and prompt-report rules;
- the canister recording the exchange as the session's first activity;
- that a snapshot load can rewind the feed's head;
- the residual eviction race at a capacity cap, with its trade-off and
a new open item.
- The runbook's purge commands (emergency rotation, stolen identity,
Appendix B) use the required `variant { Any; Is = ... }` conditions,
with a "purge everything" row written out in full and a note on why
every field is required. Every documented argument type-checks
against the .did the way dfx annotates it.
- The design record explains the filter (4.3), qualifies the evolution
rules for later filter conditions, names the new test (11), and adds
an open item on conditions added as `opt` fields (13).
- Design record: the interface conventions (one record in, a result out),
the query/update split with the *_non_replicated hint twins (D4, 6.3,
6.4, the redeem), error records with a shared code space and their
evolution rule, the activity-report terms and the renamed rules, the
metric labels, the tests, and a new 8.5 with Mermaid sequence diagrams
of a sign-in across four nodes, the sync loop with a read-through, and
activity reports with the idle purge and eviction.
- Runbook: admin_set_key_check takes the line shared-key-check prints as
is (replace = opt true to change a check), admin_retire_kid and the
status reads take records, errors carry err_case and err_code, and
Appendix A prints the new form (checked byte for byte against the
binary's golden output). Every documented argument was typed against
the .did as dfx sends it.
- README, CONTRIBUTING and the native deploy notes follow.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The recovery procedure filters admin runs by creation time, potentially omitting post-snapshot changes and undoing security actions such as leaked-key retirement or compromised-node removal.
- The prompt-report threshold allows two clock skews (design 4.4), and
the timing assertion says so.
- Purge filters: created_from and created_to are inclusive (design 4.3,
runbook Appendix B).
- Hints never revive a key whose deletion a node has read from an update
(D4); the token exchange's retry boundary is spelled out (7).
- After a snapshot load, the purges that revoked sessions (a stolen
identity's, a leaked key's) are replayed at once with the retirements
and the allow-list, not with the other purges (design 8.4, runbook
8.10); a code the load unconsumed is useless by then.
- Runbook 8.10: replay the admin runs whose apply job ran after the
snapshot, in any attempt, ordered by that time, not the runs created
after it: a run approved or re-run later makes its call after its
creation. 2.3 shows how to find when each attempt made its call.
- README: the key-check publish command targets mainnet (--network ic),
and a call made by hand belongs in the admin journal.
- CONTRIBUTING: the crates.io placeholder needs a main that carries 0.7.0;
before it the crate says publish = false.
- Directory submission: only new client registrations go unstored; the
ones made before sharing stay in oauth-clients.json.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The shared-session procedures depend on missing implementation and deployment tooling, while the publishing instructions omit the manifest change needed to remove the blocking publish = false setting.
No open issues 🎉
2 previously missed issues
In code that hasn't changed since last review
Shared-session example references unimplemented APIs
README.md:1255
This example cannot compile against the checked-in crate: src/lib.rs exports none of the shared-session types and has no McpServer::with_shared_sessions. The same missing prerequisites affect the CLI checks (README.md:585–587), deployment switch (deploy/native/README.md:343) and test suites (CONTRIBUTING.md:81–84), so these instructions cannot enable or validate sharing. Land the implementation and version bump first, or consistently mark the affected sections as future guidance rather than executable instructions.
Document allow-listed users’ access to sealed records and reports
docs/shared-sessions-operations.md:1072
Under the documented API, an allow-listed identity can also download sealed records and their plaintext metadata, and submit touch_sessions reports for other consumed sessions. This can prevent idle cleanup, while copied records become decryptable if their sealing key later leaks. Document these capabilities instead of “nothing worse” so incident responders can assess the exposure accurately.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The runbook presents provisioning and recovery procedures as executable, but the required canister implementation, build scripts, deployment workflow, and release assets are absent.
No open issues 🎉
Review effort: Max
This branch has not been deployed
No deployments
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
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.
Docs for session sharing