kayg/calternal

Watch

Fork 0

MCP design review: curate the agent surface, fix annotations, and build #491 Events on top #999

New issue

Open

opened 2026-10-03 08:52:05 +00:00 by kayg · 5 comments

kayg commented

2026-10-03 08:52:05 +00:00

Owner

Summary

A design review of the calternal MCP server, checked against the MCP and agent-tool design guidance and the draft MCP Events extension that ChatGPT now supports. Live probe on 2026-10-03 against calternal.cloud (system_info commit c4a61e8cf) using a full-access MCP App Password, plus crates/calternal-server/src/mcp.rs and contracts/actions.json on dev.

The plumbing is good: one registry, the same route guards for every adapter, bounded dispatch, ETags, per-surface switches, App Password scopes and Home prefixes, and a durable per-User change feed. The problem is the design choice above the plumbing. #484 reads "parity" as one MCP tool per HTTP route. Every guide below says not to do that, and most of the open MCP issues (#817, #818, #821, #833, #836, #515) are symptoms of it. This issue does not repeat those. It covers the design calls they do not make, and how #491 (MCP Events) should be built on top of them.

Findings

1. The tool surface is a REST mirror (high)

tools/list for a data-scope App Password returns 290 tools: 276 generated calternal_api_* plus 14 legacy calternal_*. Admin tools are filtered. 39 admin tools would also be listed for an admin credential.
The generated definitions alone are about 136 KB, roughly 34k tokens, sent before the user says anything. (#515 measured 8 tools at 654 tokens. Its "400+ tools" case is now here.)
Anthropic's tool guidance: "more tools don't always lead to better outcomes", don't wrap every API endpoint, build a few tools aimed at specific high-impact workflows. The awesome-mcp-best-practices list says the same: "Don't Map 1:1 APIs to Tools".
With this many near-synonyms (see 5), the model has to choose between calendar_events, calendar_items, calendar_range, by_day, daily, calternal_today and notes_journal_day just to answer "what did I do today?".

Proposal: keep #484 parity for API and CLI. For MCP and WebMCP, define parity as "reachable", not "one tool per route":

A curated default toolset of about 20 to 30 outcome tools built around jobs: day or range summary (logs, tasks, events), log create/update (batch), task create/update/complete, one find (kinds filter, limit, cursor), one get by any calternal ID, event create/update/delete, mail list/read/mark, files list/read, money summary and add transaction, photo search.
The long tail through progressive discovery: calternal_actions_search(query) returns matching registry actions with their schemas, and calternal_actions_call(id, args) runs one through the same middleware. The MCP client best-practices page describes exactly this search-tools pattern for large catalogues.
Optionally, an App Password toggle for "expose full registry as tools" for power clients.

2. Tools that should never be on an agent surface (high)

These are in tools/list today for my MCP App Password:

Account and auth ceremonies: assert_start/finish, add_start, remove_start/finish (WebAuthn, which an agent cannot complete), oidc_link_start/reauth_start/unlink, recovery_key, revoke_all, revoke_session, sign_out, cli_logout, create_app_password, create_app_password_profiles, download_app_password_profile, save_credential/remove_credential, rename_passkey, update_profile. An agent minting App Passwords or reading a recovery key is credential persistence, even if the route currently denies it.
Guest/public-link endpoints: public_*, edit_read/edit_session/edit_write, entries, info, linked_note, public_linked_notes, calendar_public_feed. These are for anonymous visitors holding a slug, not for the owner's agent.
Transport plumbing: Tus files_create_upload/head/patch/terminate, photos_create_upload (base64 bodies), serve, public_thumb, appearance_unsplash_thumbnail, all video_hls_* and video_source_by_item, and SSE wrappers (events, change_wakeups, notes_events, my_job_events, stream_turn). These return raw event: ...\ndata: ... text inside a poll_ms window.
UI state: record_search_opened, preferences_mode_order_*, appearance_*, files_set_pins.

Also, the server instructions say "App Passwords cannot manage accounts or administration", yet all 29 account-scope tools are listed to App Password sessions. The model is told one thing and shown another (related #774, #836 item 3).

Fix: add a surfaces exclusion by category in the registry (auth ceremony, guest, transport, UI telemetry), so new routes in those families are off MCP and WebMCP by default. List them in docs/parity-exceptions.json with the reason.

3. Annotations are derived from the HTTP method (high)

generated_tool_definitions sets only read_only and destructive, and in actions.json those are exactly method == GET. Result:

189 tools are destructiveHint: true, including notes_journal_create_log, create_note, tasks_create, notes_composer_parse, parse, preview_rename, notifications_preview_block_reminder and mail_test_connection. Several of these are pure functions (#754).
The legacy calternal_mail_reader, mostly reads, is hard-coded read_only_hint = false, destructive_hint = true. So is calternal_calendar.
idempotentHint, openWorldHint and title are never set. Spec defaults then claim every write is non-idempotent and every tool is open-world.

Effect: hosts that gate on these hints (ChatGPT, Claude) prompt on nearly everything. Users get confirmation fatigue and click "always allow", and then the real destructive tools (empty_trash, revoke_all, *_delete_*) lose their protection. The MCP blog post on annotations treats them as risk vocabulary for exactly this kind of host policy.

Fix: a reviewed, checked-in policy table per action with all four hints plus title, and a test that tools/list matches it.

Additive creates are destructive: false.
PUT with If-Match is idempotent: true.
openWorld: true only where calternal talks to the outside: mail sync/send/test, calendar subscription fetch, bookmark clip fetch, Unsplash.
Split mixed read/write legacy tools (mail_reader, calendar) so the read half can be readOnlyHint: true.

4. Names and descriptions are written for Rust developers, not models (medium)

#821 covers vague names. In addition:

Doc comments leak into tool descriptions. Examples: appearance_put: "Box nested futures so debug workers do not carry the whole Files chain (#422)". appearance_get: "Migrate legacy background files before reading saved IDs...". change_head: "A rescan uses this high-water mark before it lists the Home...". update_body: "OpenAPI declares the revision header for every adapter (#484, DESIGN §9)". Describe the tool from the caller's side, and keep #nnn/§nn out of model-facing text.
Name length. calternal_api_ spends 14 characters. With a client prefix such as mcp__Calternal__, calternal_api_notifications_preview_block_reminder (50 chars) goes past the common 64-character limit, and Aside had to truncate it to ..._preview_blo_23e9f90e. Drop api_ and use calternal_<noun>_<verb>.

5. Duplicate tools for the same job (medium)

These pairs are listed side by side, often with different argument names:

calternal_create_note / calternal_api_create_note
calternal_create_task / calternal_api_tasks_create
calternal_tick_task / calternal_api_tick
calternal_search / calternal_api_search
calternal_open / calternal_api_get_note
calternal_create_log / calternal_api_notes_journal_create_log
calternal_duplicate_item / calternal_api_calendar_duplicate_item
calternal_list_files / calternal_api_list
calternal_get_files_preferences / calternal_api_files_get_preferences (same for set_)

#817 covers their contracts drifting apart. The design point is that only one of each pair should be in tools/list. Keep aliases callable for compatibility, but do not advertise them.

6. Schemas leak HTTP structure (medium)

Generated inputs are {path, query, headers, body} groups. The model has to know that headers.If-Match carries a revision, that Tus needs Tus-Resumable: 1.0.0, and so on. Flatten to semantic arguments (id, revision, title) and map them to HTTP in the adapter.
Legacy tools put enums behind $ref: #/$defs/... (CalendarAction, MailReaderAction, DuplicateItemKind). In my client the allowed values were not visible to the model. calternal_calendar(action: "list") failed with "must be equal to one of the allowed values" and did not say which values. Inline the enums and name the values in the description.

7. Responses are not shaped for a context window (medium)

#836 covers structured output. In addition:

calternal_search("meeting") returned 41 results with no limit, kind or cursor input. Mail and Files hits have score: null and come before the scored Log hits. Promotional mail ranks first. IDs expose internal storage (users/<uuid>/Notes/...#^...). href is relative.
calendar_range returns every category as an empty array for empty days.
Binary reads come back as base64 text instead of image content or resource_link (see also #760).
Fix: limit with a cursor on every list, kinds filters, response_format: concise | detailed (Anthropic's recommendation), absolute deep links (https://calternal.cloud/d/2026-10-03#^...), stable public IDs, and resource links for files and notes.

8. Errors do not help the model recover (low, mostly covered)

Covered by #818 and #836: a 404 becomes JSON-RPC -32602 "The API route returned HTTP 404" instead of a tool result with isError: true.

One addition: calternal_open("does-not-exist") says only "Invalid Note ID". It should give the expected form (path:Notes/...) and name the tool that produces IDs.

POST /mcp without credentials returns 401 {"code":"unauthenticated"} with no WWW-Authenticate header. /.well-known/oauth-protected-resource returns the SPA HTML with a 200 instead of RFC 9728 metadata or a 404. Hosted connectors cannot discover OAuth, so today only clients that accept a pasted bearer token work. #836 item 4 tracks this. It is a hard prerequisite for #491, because ChatGPT plugins need it before events matter.

10. Server instructions are one inaccurate sentence (medium)

get_info says the tools are "thin adapters over the calternal HTTP API", which is the problem from finding 1. The instructions should give the model:

A short data-model primer: Home, Daily note, Log entry and its ^block ID, Task, Event, Note IDs, and revisions/ETags.
Time-zone rules.
Which tool to start with for common jobs.
The untrusted-content rule from #746.

MCP Events (#491): design notes

#491's delivery and security list matches the OpenAI guide well: webhook only, Standard Webhooks signing, challenge verification, SSRF guard, 256 KiB limit, and stop on 410/413. Points it misses or gets wrong:

Don't generate events from the action registry. Most events in #491 are not side effects of a user action. mail.received comes from IMAP sync. task.due/overdue, calendar.event_starting, reminder.fired and money.bill_due come from the scheduler. share.* comes from another User. Keep a small, hand-reviewed event catalogue with reviewed payloadSchemas, fed by (a) the change feed and (b) the scheduler. Generating events would also inherit findings 3 and 4.
Use the change feed you already have for cursors. /files/changes already has a per-User monotonic cursor, change_head and a floor. That gives real cursor values and honest truncated: true when a cursor falls below the floor. Most servers can only return cursor: null. The feed is file-level (write Notes/20261003-dailynote.md), so it needs a domain projection (log.created, task.completed). Make eventId stable across retries, for example <feed cursor>:<event>:<block id>, or the mail UIDVALIDITY:UID.
Keep payloads minimal and pair each with a read tool. The draft and the OpenAI guide both say to send triage fields and IDs, then fetch details through a tool. For example, mail.received carries {account_id, message_id, thread_id, from, subject, received_at, url} and the agent reads the body with the mail read tool. This only works well once the curated read tools from finding 1 exist. Never put instructions in payloads (#746).
Prevent feedback loops. The OpenAI guide asks you to test this. "On log.created, add a tag" re-fires forever. Put actor: {kind, app_password_id} in every payload, and add a default-on subscription filter exclude_own_writes.
Make write tools idempotent. Events can arrive duplicated and out of order, and agents retry. create_log, tasks_create and money_create_transaction take no idempotency key. Add an optional idempotency_key (agents can pass the eventId). Related #814.
Treat a subscription as a credential. Bind it to the App Password ID as well as the User. Revoking the password, turning off the MCP switch, or narrowing the scopes or Home prefix stops delivery. Events outside a Home-limited password's prefix are never sent, and a data-scope password cannot subscribe to account events. ChatGPT ignores terminated, so stop delivery and answer the next refresh with -32012 Forbidden. Grant a finite TTL (days, not null) and store subscriptions in SQLite so they survive restarts, as the guide requires.
Protocol version. MCP_PROTOCOLS already lists 2026-07-28, but get_info enables only tools. Confirm that rmcp answers server/discover with capabilities.events and routes events/* on the same authenticated endpoint.
Start small. v1: mail.received, log.created, task.due, calendar.event_starting, file.added (folder filter). Money and share events come after the payloads and filters prove out. Filters stay simple key-value, as the draft says: no query language.

Suggested order

Category exclusions (2) and the annotation policy table (3). Both are cheap and have the biggest safety effect.
Curated toolset plus actions_search/actions_call (1), and hiding duplicate tools (5). Measure tools/list tokens before and after (#515).
Description and name pass (4), schema flattening (6), response shaping (7) and instructions (10).
OAuth discovery (9), then #491 on top of the curated read tools and the change feed.

Sources

Anthropic, Writing effective tools for agents: https://www.anthropic.com/engineering/writing-tools-for-agents
MCP blog, Tool Annotations as Risk Vocabulary (2026-03-16): https://blog.modelcontextprotocol.io/posts/2026-03-16-tool-annotations
MCP client best practices (tool search / progressive discovery): https://modelcontextprotocol.io/docs/develop/clients/client-best-practices
anthropics/skills mcp-builder best practices: https://github.com/anthropics/skills/blob/main/skills/mcp-builder/reference/mcp_best_practices.md
awslabs MCP design guidelines (64-char names): https://github.com/awslabs/mcp/blob/main/DESIGN_GUIDELINES.md
OpenAI, MCP Events in ChatGPT: https://developers.openai.com/plugins/build/mcp-events
MCP Events design sketch (draft, 2026-02-19): https://github.com/modelcontextprotocol/experimental-ext-triggers-events/blob/main/docs/design-sketch-proposal.md
MCP Triggers and Events WG charter: https://modelcontextprotocol.io/community/working-groups/triggers-events
WorkOS, "An event subscription is a credential" (2026-10-01): https://workos.com/blog/mcp-events-chatgpt-subscription-revocation

Related: #484, #491, #515, #746, #754, #760, #774, #789, #814, #817, #818, #821, #833, #836

## Summary A design review of the calternal MCP server, checked against the MCP and agent-tool design guidance and the draft MCP Events extension that ChatGPT now supports. Live probe on 2026-10-03 against calternal.cloud (`system_info` commit `c4a61e8cf`) using a full-access MCP App Password, plus `crates/calternal-server/src/mcp.rs` and `contracts/actions.json` on `dev`. The plumbing is good: one registry, the same route guards for every adapter, bounded dispatch, ETags, per-surface switches, App Password scopes and Home prefixes, and a durable per-User change feed. The problem is the design choice above the plumbing. **#484 reads "parity" as one MCP tool per HTTP route.** Every guide below says not to do that, and most of the open MCP issues (#817, #818, #821, #833, #836, #515) are symptoms of it. This issue does not repeat those. It covers the design calls they do not make, and how #491 (MCP Events) should be built on top of them. ## Findings ### 1. The tool surface is a REST mirror (high) - `tools/list` for a data-scope App Password returns **290 tools**: 276 generated `calternal_api_*` plus 14 legacy `calternal_*`. Admin tools are filtered. 39 admin tools would also be listed for an admin credential. - The generated definitions alone are about **136 KB, roughly 34k tokens**, sent before the user says anything. (#515 measured 8 tools at 654 tokens. Its "400+ tools" case is now here.) - Anthropic's tool guidance: "more tools don't always lead to better outcomes", don't wrap every API endpoint, build a few tools aimed at specific high-impact workflows. The awesome-mcp-best-practices list says the same: "Don't Map 1:1 APIs to Tools". - With this many near-synonyms (see 5), the model has to choose between `calendar_events`, `calendar_items`, `calendar_range`, `by_day`, `daily`, `calternal_today` and `notes_journal_day` just to answer "what did I do today?". **Proposal:** keep #484 parity for API and CLI. For MCP and WebMCP, define parity as **"reachable"**, not "one tool per route": - A **curated default toolset of about 20 to 30 outcome tools** built around jobs: day or range summary (logs, tasks, events), log create/update (batch), task create/update/complete, one `find` (kinds filter, limit, cursor), one `get` by any calternal ID, event create/update/delete, mail list/read/mark, files list/read, money summary and add transaction, photo search. - The **long tail through progressive discovery**: `calternal_actions_search(query)` returns matching registry actions with their schemas, and `calternal_actions_call(id, args)` runs one through the same middleware. The MCP client best-practices page describes exactly this search-tools pattern for large catalogues. - Optionally, an App Password toggle for "expose full registry as tools" for power clients. ### 2. Tools that should never be on an agent surface (high) These are in `tools/list` today for my MCP App Password: - **Account and auth ceremonies:** `assert_start/finish`, `add_start`, `remove_start/finish` (WebAuthn, which an agent cannot complete), `oidc_link_start/reauth_start/unlink`, `recovery_key`, `revoke_all`, `revoke_session`, `sign_out`, `cli_logout`, `create_app_password`, `create_app_password_profiles`, `download_app_password_profile`, `save_credential/remove_credential`, `rename_passkey`, `update_profile`. An agent minting App Passwords or reading a recovery key is credential persistence, even if the route currently denies it. - **Guest/public-link endpoints:** `public_*`, `edit_read/edit_session/edit_write`, `entries`, `info`, `linked_note`, `public_linked_notes`, `calendar_public_feed`. These are for anonymous visitors holding a slug, not for the owner's agent. - **Transport plumbing:** Tus `files_create_upload/head/patch/terminate`, `photos_create_upload` (base64 bodies), `serve`, `public_thumb`, `appearance_unsplash_thumbnail`, all `video_hls_*` and `video_source_by_item`, and SSE wrappers (`events`, `change_wakeups`, `notes_events`, `my_job_events`, `stream_turn`). These return raw `event: ...\ndata: ...` text inside a `poll_ms` window. - **UI state:** `record_search_opened`, `preferences_mode_order_*`, `appearance_*`, `files_set_pins`. Also, the server instructions say "App Passwords cannot manage accounts or administration", yet all 29 `account`-scope tools are listed to App Password sessions. The model is told one thing and shown another (related #774, #836 item 3). **Fix:** add a `surfaces` exclusion **by category** in the registry (auth ceremony, guest, transport, UI telemetry), so new routes in those families are off MCP and WebMCP by default. List them in `docs/parity-exceptions.json` with the reason. ### 3. Annotations are derived from the HTTP method (high) `generated_tool_definitions` sets only `read_only` and `destructive`, and in `actions.json` those are exactly `method == GET`. Result: - **189 tools are `destructiveHint: true`**, including `notes_journal_create_log`, `create_note`, `tasks_create`, `notes_composer_parse`, `parse`, `preview_rename`, `notifications_preview_block_reminder` and `mail_test_connection`. Several of these are pure functions (#754). - The legacy `calternal_mail_reader`, mostly reads, is hard-coded `read_only_hint = false, destructive_hint = true`. So is `calternal_calendar`. - `idempotentHint`, `openWorldHint` and `title` are never set. Spec defaults then claim every write is non-idempotent and every tool is open-world. Effect: hosts that gate on these hints (ChatGPT, Claude) prompt on nearly everything. Users get confirmation fatigue and click "always allow", and then the real destructive tools (`empty_trash`, `revoke_all`, `*_delete_*`) lose their protection. The MCP blog post on annotations treats them as risk vocabulary for exactly this kind of host policy. **Fix:** a reviewed, checked-in policy table per action with all four hints plus `title`, and a test that `tools/list` matches it. - Additive creates are `destructive: false`. - `PUT` with `If-Match` is `idempotent: true`. - `openWorld: true` only where calternal talks to the outside: mail sync/send/test, calendar subscription fetch, bookmark clip fetch, Unsplash. - Split mixed read/write legacy tools (`mail_reader`, `calendar`) so the read half can be `readOnlyHint: true`. ### 4. Names and descriptions are written for Rust developers, not models (medium) #821 covers vague names. In addition: - **Doc comments leak into tool descriptions.** Examples: `appearance_put`: "Box nested futures so debug workers do not carry the whole Files chain (#422)". `appearance_get`: "Migrate legacy background files before reading saved IDs...". `change_head`: "A rescan uses this high-water mark before it lists the Home...". `update_body`: "OpenAPI declares the revision header for every adapter (#484, DESIGN §9)". Describe the tool from the caller's side, and keep `#nnn`/`§nn` out of model-facing text. - **Name length.** `calternal_api_` spends 14 characters. With a client prefix such as `mcp__Calternal__`, `calternal_api_notifications_preview_block_reminder` (50 chars) goes past the common 64-character limit, and Aside had to truncate it to `..._preview_blo_23e9f90e`. Drop `api_` and use `calternal_<noun>_<verb>`. ### 5. Duplicate tools for the same job (medium) These pairs are listed side by side, often with different argument names: - `calternal_create_note` / `calternal_api_create_note` - `calternal_create_task` / `calternal_api_tasks_create` - `calternal_tick_task` / `calternal_api_tick` - `calternal_search` / `calternal_api_search` - `calternal_open` / `calternal_api_get_note` - `calternal_create_log` / `calternal_api_notes_journal_create_log` - `calternal_duplicate_item` / `calternal_api_calendar_duplicate_item` - `calternal_list_files` / `calternal_api_list` - `calternal_get_files_preferences` / `calternal_api_files_get_preferences` (same for `set_`) #817 covers their contracts drifting apart. The design point is that **only one of each pair should be in `tools/list`**. Keep aliases callable for compatibility, but do not advertise them. ### 6. Schemas leak HTTP structure (medium) - Generated inputs are `{path, query, headers, body}` groups. The model has to know that `headers.If-Match` carries a revision, that Tus needs `Tus-Resumable: 1.0.0`, and so on. Flatten to semantic arguments (`id`, `revision`, `title`) and map them to HTTP in the adapter. - Legacy tools put enums behind `$ref: #/$defs/...` (`CalendarAction`, `MailReaderAction`, `DuplicateItemKind`). In my client the allowed values were not visible to the model. `calternal_calendar(action: "list")` failed with "must be equal to one of the allowed values" and did not say which values. Inline the enums and name the values in the description. ### 7. Responses are not shaped for a context window (medium) #836 covers structured output. In addition: - `calternal_search("meeting")` returned **41 results with no `limit`, `kind` or cursor input**. Mail and Files hits have `score: null` and come before the scored Log hits. Promotional mail ranks first. IDs expose internal storage (`users/<uuid>/Notes/...#^...`). `href` is relative. - `calendar_range` returns every category as an empty array for empty days. - Binary reads come back as base64 text instead of image content or `resource_link` (see also #760). - **Fix:** `limit` with a cursor on every list, `kinds` filters, `response_format: concise | detailed` (Anthropic's recommendation), absolute deep links (`https://calternal.cloud/d/2026-10-03#^...`), stable public IDs, and resource links for files and notes. ### 8. Errors do not help the model recover (low, mostly covered) Covered by #818 and #836: a 404 becomes JSON-RPC `-32602 "The API route returned HTTP 404"` instead of a tool result with `isError: true`. One addition: `calternal_open("does-not-exist")` says only "Invalid Note ID". It should give the expected form (`path:Notes/...`) and name the tool that produces IDs. ### 9. Remote sign-in discovery blocks ChatGPT and Claude connectors (high for #491) `POST /mcp` without credentials returns `401 {"code":"unauthenticated"}` **with no `WWW-Authenticate` header**. `/.well-known/oauth-protected-resource` returns the SPA HTML with a 200 instead of RFC 9728 metadata or a 404. Hosted connectors cannot discover OAuth, so today only clients that accept a pasted bearer token work. #836 item 4 tracks this. **It is a hard prerequisite for #491**, because ChatGPT plugins need it before events matter. ### 10. Server instructions are one inaccurate sentence (medium) `get_info` says the tools are "thin adapters over the calternal HTTP API", which is the problem from finding 1. The instructions should give the model: - A short data-model primer: Home, Daily note, Log entry and its `^block` ID, Task, Event, Note IDs, and revisions/ETags. - Time-zone rules. - Which tool to start with for common jobs. - The untrusted-content rule from #746. ## MCP Events (#491): design notes #491's delivery and security list matches the OpenAI guide well: webhook only, Standard Webhooks signing, challenge verification, SSRF guard, 256 KiB limit, and stop on 410/413. Points it misses or gets wrong: 1. **Don't generate events from the action registry.** Most events in #491 are not side effects of a user action. `mail.received` comes from IMAP sync. `task.due/overdue`, `calendar.event_starting`, `reminder.fired` and `money.bill_due` come from the scheduler. `share.*` comes from another User. Keep a small, hand-reviewed event catalogue with reviewed `payloadSchema`s, fed by (a) the change feed and (b) the scheduler. Generating events would also inherit findings 3 and 4. 2. **Use the change feed you already have for cursors.** `/files/changes` already has a per-User monotonic cursor, `change_head` and a floor. That gives real `cursor` values and honest `truncated: true` when a cursor falls below the floor. Most servers can only return `cursor: null`. The feed is file-level (`write Notes/20261003-dailynote.md`), so it needs a domain projection (`log.created`, `task.completed`). Make `eventId` stable across retries, for example `<feed cursor>:<event>:<block id>`, or the mail UIDVALIDITY:UID. 3. **Keep payloads minimal and pair each with a read tool.** The draft and the OpenAI guide both say to send triage fields and IDs, then fetch details through a tool. For example, `mail.received` carries `{account_id, message_id, thread_id, from, subject, received_at, url}` and the agent reads the body with the mail read tool. This only works well once the curated read tools from finding 1 exist. Never put instructions in payloads (#746). 4. **Prevent feedback loops.** The OpenAI guide asks you to test this. "On `log.created`, add a tag" re-fires forever. Put `actor: {kind, app_password_id}` in every payload, and add a default-on subscription filter `exclude_own_writes`. 5. **Make write tools idempotent.** Events can arrive duplicated and out of order, and agents retry. `create_log`, `tasks_create` and `money_create_transaction` take no idempotency key. Add an optional `idempotency_key` (agents can pass the `eventId`). Related #814. 6. **Treat a subscription as a credential.** Bind it to the App Password ID as well as the User. Revoking the password, turning off the MCP switch, or narrowing the scopes or Home prefix stops delivery. Events outside a Home-limited password's prefix are never sent, and a data-scope password cannot subscribe to account events. ChatGPT ignores `terminated`, so stop delivery and answer the next refresh with `-32012 Forbidden`. Grant a finite TTL (days, not `null`) and store subscriptions in SQLite so they survive restarts, as the guide requires. 7. **Protocol version.** `MCP_PROTOCOLS` already lists `2026-07-28`, but `get_info` enables only tools. Confirm that rmcp answers `server/discover` with `capabilities.events` and routes `events/*` on the same authenticated endpoint. 8. **Start small.** v1: `mail.received`, `log.created`, `task.due`, `calendar.event_starting`, `file.added` (folder filter). Money and share events come after the payloads and filters prove out. Filters stay simple key-value, as the draft says: no query language. ## Suggested order 1. Category exclusions (2) and the annotation policy table (3). Both are cheap and have the biggest safety effect. 2. Curated toolset plus `actions_search`/`actions_call` (1), and hiding duplicate tools (5). Measure `tools/list` tokens before and after (#515). 3. Description and name pass (4), schema flattening (6), response shaping (7) and instructions (10). 4. OAuth discovery (9), then #491 on top of the curated read tools and the change feed. ## Sources - Anthropic, Writing effective tools for agents: https://www.anthropic.com/engineering/writing-tools-for-agents - MCP blog, Tool Annotations as Risk Vocabulary (2026-03-16): https://blog.modelcontextprotocol.io/posts/2026-03-16-tool-annotations - MCP client best practices (tool search / progressive discovery): https://modelcontextprotocol.io/docs/develop/clients/client-best-practices - anthropics/skills mcp-builder best practices: https://github.com/anthropics/skills/blob/main/skills/mcp-builder/reference/mcp_best_practices.md - awslabs MCP design guidelines (64-char names): https://github.com/awslabs/mcp/blob/main/DESIGN_GUIDELINES.md - OpenAI, MCP Events in ChatGPT: https://developers.openai.com/plugins/build/mcp-events - MCP Events design sketch (draft, 2026-02-19): https://github.com/modelcontextprotocol/experimental-ext-triggers-events/blob/main/docs/design-sketch-proposal.md - MCP Triggers and Events WG charter: https://modelcontextprotocol.io/community/working-groups/triggers-events - WorkOS, "An event subscription is a credential" (2026-10-01): https://workos.com/blog/mcp-events-chatgpt-subscription-revocation Related: #484, #491, #515, #746, #754, #760, #774, #789, #814, #817, #818, #821, #833, #836

kayg commented

2026-10-03 09:00:54 +00:00

Author

Owner

Started independent read-only re-critique for #999 on the existing dev checkout. HEAD/base and origin/dev: f06679b11c. Reviewed 7a head: 516faaa698. No checkout, merge, code change, commit, production probe or runtime certification is planned. Source counts already distinguish 290 tools on dev from 295 on 7a; checking primary specifications and all cited issue threads before the final reply.

Started independent read-only re-critique for #999 on the existing dev checkout. HEAD/base and origin/dev: f06679b11cde29cc0b7120fdab5f721389caf695. Reviewed 7a head: 516faaa698570bdb468626cf6cd75d9c81b33ac2. No checkout, merge, code change, commit, production probe or runtime certification is planned. Source counts already distinguish 290 tools on dev from 295 on 7a; checking primary specifications and all cited issue threads before the final reply.

kayg commented

2026-10-03 09:40:13 +00:00

Author

Owner

Source findings for the re-critique (not runtime certification):

At dev f06679b11 and 7a 516faaa69, the non-admin generated definitions contain 153 and 156 explicit destructive hints. Five legacy tools explicitly set the hint; nine omit annotations. The non-admin totals are 158/161 explicit or 167/170 with MCP defaults. The issue's 189 is not reproduced at these heads.
At 7a, mcp.rs:1355 checks only initialize before adding the Events capability. get_info at :1240 still enables tools only; rmcp 3.5.0's default discover uses get_info. This differs from OpenAI's current server/discover Events discovery guidance.
At 7a, mcp_events.rs:130 keeps grants in a HashMap; docs/mcp-events.md says restart clears them. Current OpenAI guidance requires retaining a grant across restart for its granted lifetime.
At 7a, contracts/openapi.json has 340 supported-method operations but contracts/actions.json has 338. mcp_event_subscriptions and mcp_event_revoke are missing from the registry. Offline classification passes, so classification alone does not prove complete registry parity.
Staging only: unauthenticated POST /mcp returns 401 application/json without WWW-Authenticate. GET /.well-known/oauth-protected-resource returns 200 text/html. No credentials were loaded, no authenticated calls were made, and production was not contacted.

The final reply will classify every original finding, separate owner decisions from defects, and propose implementation slices without changing code.

Source findings for the re-critique (not runtime certification): - At dev f06679b11 and 7a 516faaa69, the non-admin generated definitions contain 153 and 156 explicit destructive hints. Five legacy tools explicitly set the hint; nine omit annotations. The non-admin totals are 158/161 explicit or 167/170 with MCP defaults. The issue's 189 is not reproduced at these heads. - At 7a, mcp.rs:1355 checks only initialize before adding the Events capability. get_info at :1240 still enables tools only; rmcp 3.5.0's default discover uses get_info. This differs from OpenAI's current server/discover Events discovery guidance. - At 7a, mcp_events.rs:130 keeps grants in a HashMap; docs/mcp-events.md says restart clears them. Current OpenAI guidance requires retaining a grant across restart for its granted lifetime. - At 7a, contracts/openapi.json has 340 supported-method operations but contracts/actions.json has 338. mcp_event_subscriptions and mcp_event_revoke are missing from the registry. Offline classification passes, so classification alone does not prove complete registry parity. - Staging only: unauthenticated POST /mcp returns 401 application/json without WWW-Authenticate. GET /.well-known/oauth-protected-resource returns 200 text/html. No credentials were loaded, no authenticated calls were made, and production was not contacted. The final reply will classify every original finding, separate owner decisions from defects, and propose implementation slices without changing code.

kayg commented

2026-10-03 10:22:05 +00:00

Author

Owner

Independent re-critique of #999

Keep the shared registry and route authorization. Fix discovery, annotations, contracts and Events delivery first. A smaller default catalogue is a product choice to test, not a conclusion proved by the tool count. Several proposed exclusions would remove capabilities the owner explicitly required in #484.

This is a read-only review on 2026-10-03. I fetched origin once and inspected immutable snapshots:

origin/dev: f06679b11cde29cc0b7120fdab5f721389caf695 (also checkout HEAD).
origin/job/merge-round-7a: 516faaa698570bdb468626cf6cd75d9c81b33ac2.

Evidence below uses dev file lines unless marked 7a. Both heads have the same underlying tool-contract defects discussed below; 7a also has Events and agent setup documents. I read the bodies and all comments of #484, #491, #515, #754, #774, #817, #818, #821, #833, #836 and #472. Work reported on other job branches is not assumed to be merged. For example, #754/#818/#821/#836 comments report fixes that are absent from these snapshots. Coordinate with those jobs rather than repeat their work.

Document correction: DESIGN §9 is Notes and the editor, not the parity decision. #484 states the parity rule; DESIGN §41 states the thin-adapter rule. §55 sets Events decisions. §58 exists on 7a, but not this dev head. There is no §59 in either reviewed DESIGN file. §60's one-event-path rule applies to Canvas collaboration writes; it must not be interpreted as a ban on a webhook delivery queue.

Reproduced counts

I reconstructed the generated definitions from contracts/actions.json using the fields emitted by mcp.rs:338. This is a deterministic source measurement, not a captured live tools/list response. JSON is compact UTF-8. Tokens use the repository benchmark's pinned tiktoken==0.14.0. The local reproduction is artifacts/mcp-recritique-999/counts.py.

Measurement	dev	7a
Registry actions	333	338
Eligible generated MCP tools	315	320
Non-admin generated tools	276	281
Legacy tools	14	14
Non-admin listed tools	290	295
Additional admin generated tools	39	39
Listed generated actions labelled account	29	29
Non-admin generated definitions, bytes, with annotations	152,170	155,690
Same definitions, `o200k_base` tokens	34,127	34,892
Same definitions, `cl100k_base` tokens	32,469	33,212
Definition bytes with annotations removed	135,610	138,830
Non-admin generated explicit destructive hints	153	156
Explicit destructive hints including legacy	158	161
Destructive hints including legacy spec defaults	167	170

The roughly 136 KB claim matches dev definitions without annotations. The actual reconstructed annotated array is 152 KB and 34,127 o200k_base tokens, before legacy definitions and the result envelope. The issue's 189 destructive tools is not reproducible at either reviewed head. All generated tools, including admin, have 177/180 explicit destructive hints. Five legacy tools explicitly set them and nine omit annotations; all fourteen legacy tools have effective readOnlyHint=false and destructiveHint=true under the defaults. This includes the read-only legacy Search, Open, Today, Files list and Files preference read tools.

Findings 1–10

1. REST mirror — CONFIRMED; proposed replacement — PARTLY

scripts/action_registry.py:151 advertises every supported non-onboarding route on all three generated adapters. mcp.rs:338 maps it to tools; mcp.rs:1198 filters only admin tools and ignores discovery cursors. The size and overlapping vocabulary are real.

The claim that the whole catalogue necessarily enters model context before the first message is client-dependent. The official MCP client guidance describes host-side discovery: fetch the tools normally, then load selected definitions into model context. It does not require every server to replace its catalogue with a generic call tool. Server-side actions_search is an optional compatibility strategy, not that exact documented mechanism.

The proposed 20–30 tools omit Contacts, Analytics, sharing and Collaborate, versions and restore, transfers/import/export, jobs, Connected Accounts, App Passwords, behavioural settings and Canvas element/export actions. Search/get cannot replace those writes. #484 explicitly includes these capabilities. Hiding them behind a dispatcher could preserve functional reachability only if discovery and execution work in the actual supported clients and the parity gate proves each action. A schema that accepts arbitrary args also loses per-action validation and clear host confirmation policy.

Recommendation: keep the complete canonical catalogue available to authorized clients; filter grants, remove redundant advertising, improve descriptions and page deterministically. Test an optional curated profile against full and host-deferred profiles. Use task success, missed capabilities, wrong writes, repair calls, total tokens and latency. Include a weaker client. Anthropic's guidance supports workflow tools and evaluation, but does not establish a universal optimum of 20–30 tools. Keep workflow wrappers generated from shared mappings; do not add separate writers.

2. Tools that should never be exposed — PARTLY

The 29 account labels and irrelevant transport/ceremony entries are confirmed. Examples: contracts/actions.json:3474 (create_app_password), :3405 (profile download), :15801 (public info), :16108 (public entries), :15754 (Tab order). But listing a tool does not prove the credential can invoke it.

MCP dispatch retains the original credential (mcp.rs:216); inner middleware checks it again and App Password context carries only data scope (wire.rs:2651). calternal-auth/src/api.rs:300 checks account/admin authority; creating an App Password also requires a fresh assertion (:1441). The public profile download is a separate bearer-capability flow explicitly bypassing normal session extraction (wire.rs:2555 vicinity), so a blanket “all account routes deny” statement is also wrong. Its token is consumed once, not a normal account read.

Separate these cases:

Hardware ceremonies and pure presentation can use existing documented exemptions. Do not pretend an agent can complete a passkey ceremony alone.
Credential minting/recovery should be absent for data-only credentials. For an authorized Installation, #484 requires account parity. Removing it everywhere requires an owner decision and a safe human-mediated completion path.
Public-link owner management is a User action; public visitor transport is a different capability. Remove raw visitor plumbing only when a usable equivalent is named and tested.
Tus, media and SSE deserve better transfer/read/event interfaces. Deleting their tools before replacements would remove upload, playback/export or change access.
Tab order, pinning and search-open recording can change behaviour or ranking. They are not automatically pure presentation. Pure appearance has a stronger exemption case, but appearance operations can also upload or delete Home files.

Recommendation: reviewed per-action surface intent plus required scopes/freshness and an equivalent capability where relevant. Reuse docs/parity-exceptions.json; do not use a broad path/category deny that silently exempts future User actions. #774 demonstrates why labels must describe actual guards. Discovery filtering is useful but cannot replace the guards.

3. Method-derived annotations — CONFIRMED; details and fix — PARTLY

The generator uses GET/HEAD plus the ZIP-download POST exception, not exactly GET (action_registry.py:146). Generated MCP emits only read/destructive hints (mcp.rs:352). Pure parsers are wrongly classified at contracts/actions.json:12096 and :13428; route read-access classification repeats this problem (wire.rs:2371). This is #754, not only a confirmation nuisance.

calternal_mail_reader explicitly has both false/true hints (mcp.rs:645). calternal_calendar has no annotations (:1056), which produces conservative defaults; it is not hard-coded the same way. Read-only legacy tools also lack read-only annotations, which the critique missed. Confirmation fatigue is plausible, but no host confirmation rate or User behaviour was measured here.

Use reviewed metadata in the common contract. The current MCP annotation schema defines destructive as possibly destructive updates; false means additive updates. Idempotent means repeating the same arguments adds no further effect. Both hints matter only when read-only is false. Hints are not authorization. Thus:

A create is non-destructive only if it cannot overwrite, replace or trigger destructive side effects.
If-Match prevents stale overwrites; it does not prove replay safety for audit records, versions, notices or jobs. Check effects per action. Keep replay policy separate from the hint.
openWorldHint describes an open domain of entities, not whether a network socket exists. Bounded Connected Account reads can be closed-world; arbitrary URL fetch and mail to arbitrary recipients are open-world. OpenAI's current tool guidance makes this distinction.
A title is useful display metadata, not a fifth risk dimension.

Keep unknown actions conservative, and fail review for missing declarations. Pure previews must be allowed to read-only credentials through the route policy too. Split mixed tools only when the canonical tools retain every operation and old calls retain compatibility.

4. Names/descriptions — CONFIRMED; blanket rename — PARTLY

The quoted internal comments are in the registry and are copied directly into MCP descriptions: appearance_put, appearance_get, change_head, update_body. Use the existing contracts/action-overrides.json mechanism and caller-facing route summaries. Keep useful invariants such as revision requirements; do not merely delete technical terms or references without replacing the guidance.

The longest name is 50 characters. calternal_api_ is 13, not 14; mcp__Calternal__ is 15. Together they reach 65, so the cited 64-character client limit remains plausible. MCP itself recommends up to 128 characters, so this is a client interoperability constraint, not a protocol violation. Budget names for supported hosts and preserve aliases. A global api_ deletion does not solve ambiguity and creates collisions with legacy names. #821 already has a canonical-name/alias job in progress.

5. Duplicate tools — CONFIRMED; hide all pairs — PARTLY

Legacy and generated names are registered together (mcp.rs:90). All listed pairs are present. They are often overlapping capabilities, not interchangeable aliases: one-entry Log versus batch, Home path versus item ID, different search fields, and browser navigation versus Note reading. #817's thread also records an attempted compatibility break when browser Open and Today were renamed.

Choose a canonical data contract, preserve old argument shapes in explicit wrappers, and stop advertising redundant aliases only after supported-client tests. Browser navigation remains a distinct browser capability. “Callable but not advertised” must be tested: hosts can refuse tools absent from their imported catalogue. Do not assume that strategy preserves every existing client.

6. HTTP-shaped inputs and enum references — CONFIRMED; protocol defect claim — PARTLY

The grouped inputs come from action_registry.py:106; legacy enums come from mcp.rs:368, :458, :514. The generated resolver already inlines local references (action_registry.py:69), while schemars emits the legacy references. $ref/$defs are valid JSON Schema, so the reported client losing enums is an interoperability failure, not invalid MCP schema.

Flatten selected common workflows through shared, declared mappings. Preserve required revisions, upload offsets, content types and distinction between absent and null. Keep the raw canonical mapping for expert clients. Inline legacy enum values where this helps affected clients; explain each action and return a bounded allowed-values list on enum errors. Coordinate with #818 for field paths and #833 for schema correctness before cosmetic flattening.

7. Context-sized responses — PARTLY

Legacy Search accepts only q (mcp.rs:362, :638). The API and generated action already accept a provider limit of 1–200, default 20 (main.rs:170, :705). They do not apply a global result cap: providers are concatenated in stable registry order (main.rs:1020), so 41 results and unscored hits before scored hits are plausible. “Promotional mail ranks first” and that exact result count are historical production observations, not reproduced on either head. Scores from different providers are not necessarily comparable; do not fix this by sorting nulls alone.

Binary output is encoded as a base64 string in a text JSON envelope (actions.rs:201, mcp.rs:107), confirmed. Empty category arrays are harmless overhead, not the same severity as unreachable later pages. Keep stable response shapes unless measurements justify a concise alternative.

Recommendation: deterministic bounded pages where continuation makes sense; stable IDs, revision, continuation and partial-result state must survive concise output. Search needs an explicit global budget and kind semantics without starving providers; changing its ranking belongs in Search. A limit on every list is not necessary for a fixed small enum. Use configured Instance-origin absolute deep links, not a hard-coded deployment hostname. §33 IDs must survive rename. Images can use image content; large Files need an authenticated fetch path behind resource links. A resource_link alone does not implement authorization, transfer or a client read method. Add structured results alongside legacy text, with version-appropriate schemas. MCP tool-result guidance defines these content types and the text compatibility path. Concise/detailed is optional and should be evaluated, as Anthropic recommends.

8. Errors — CONFIRMED; proposed Note ID example — WRONG

mcp.rs:321 maps HTTP 4xx to invalid-params and 5xx to internal RPC errors. Domain failure should be a failed tool result with safe typed status/code, repair guidance and retryability. Malformed RPC/tool schemas remain protocol/input errors. Include revision conflict details without reflecting secrets, raw upstream HTML or Home payloads. #818 and #836 already own these slices.

calternal_open uses Uuid::parse_str (mcp.rs:786), so path:Notes/... would still fail. Its help must name a stable Note UUID and the read/search action that returns that UUID. Search hit IDs are not automatically valid Note IDs. Browser calternal_open currently takes href and navigates (tools.ts:156); it needs separate compatibility handling.

I made two unauthenticated requests to STAGING only, dev.calternal.com. POST /mcp returned 401 JSON without WWW-Authenticate; GET /.well-known/oauth-protected-resource returned 200 HTML. No credentials were loaded. No production request was made. Source agrees: mcp.rs:1283 returns the ordinary unauthenticated response; neither head has OAuth resource metadata routes. The DAV Basic challenge is unrelated.

This blocks normal OAuth-discovered connection to private data. The MCP authorization specification requires protected-resource metadata and discovery challenges for that authorization flow. Metadata alone is insufficient: authorization service, PKCE, audience-bound tokens, scopes, consent, expiry and revocation must all work. An upstream OIDC sign-in provider is not automatically an MCP authorization server.

OAuth is a prerequisite for the targeted hosted authorization flow, not for Events as a protocol. A bearer client can subscribe and receive a signed webhook. #491's sender can be tested locally independently; hosted-client end-to-end acceptance should explicitly depend on #836. Do not delay every Events correctness fix until OAuth or curation is finished.

10. Server instructions — PARTLY

mcp.rs:1229 contains the quoted short instructions. The account prohibition is correct for App Passwords; their discovery is misleading. It must distinguish authorized Installation sessions rather than promise account tools work for every bearer credential. “Thin adapter” matches DESIGN §41 and is not itself an inaccurate design.

Add a concise vocabulary/identity/time-zone/revision primer, common first reads, partial-result rules and the untrusted-content boundary. 7a already has the public guide and Skill under §58 (agent_docs.rs:32, agent_docs/skill_intro.md); extend those shared materials instead of writing a conflicting second manual. The Skill is not automatically loaded by MCP clients. Neither guide text nor an annotation makes an embedded instruction safe.

What the critique missed

Read-only semantics are broken in the other direction too. Both snapshots' Daily GET creates and indexes a missing Daily note and updates nearby navigation (crates/plugins/notes/src/lib.rs:3855). Its generated hint is read-only. The profile-download GET consumes a one-use credential token (contracts/actions.json:3405, quoted handler contract). Reviewing POST creates alone misses these state changes. #833's later thread reports the explicit Daily create/read split on another branch; require it in the combined round. This is source evidence, not a newly reproduced runtime exploit.

The Note schema collision is an actual capability failure. Note and Task property handlers publish PropertiesPatch with different fields (notes/src/lib.rs:3621, tasks_api.rs:336, contracts/openapi.json:22031). Both snapshots still point Note edits at the colliding component. Agent-friendly names cannot fix a schema that describes the wrong write. Complete #833 before declaring semantic parity.

Credential outputs require separate treatment. App Password creation and profile download return credential material; recovery issues a key. A scoped agent must not acquire account authority from text, and a normal tool-result envelope must not send newly minted credentials into model history or logs. Use a protected User completion/delivery channel if those workflows remain available. Recovery and profile download are not ordinary read tools. Keep route freshness and authority intact; curation is not a security boundary.

Prompt injection is a cross-surface data boundary. Note bodies, Mail subjects/bodies, filenames, snippets, linked content, public content and webhook fields can contain hostile instructions. Generated results currently become plain ContentBlock::text (mcp.rs:107), without a provenance/data envelope. WebMCP's untrustedContentHint (generated.ts:85) is only a hint. Use fixed server-owned metadata and nested untrusted data, bounded excerpts and safe error details. Validate mutations against granted authority and actual User intent; never let retrieved text select a new recipient, callback, credential scope or tool policy. Provenance labels reduce confusion but cannot guarantee prevention. #774's thread reports relevant envelope work on a separate branch.

Cross-User classification is not runtime isolation proof. #472 is closed after the offline gate and live ownership checks; it did not report twenty exploitable routes. Its final thread retains unseeded fixture gaps. Current xuser_matrix.py:282 binds generated entries to classified routes, but classification does not exercise every MCP or WebMCP call. Replay A's real identities as B through legacy and canonical adapters; test admin denial, Share revocation, restricted Home/Plugin grants, account changes and cached tool lists. Derived data and event payloads need the same ownership proof. A shared cache must never key only on Role, and pagination cursors must not mix credentials or catalogue revisions.

Tool-list performance is not per-call performance. Schema construction is already cached in a OnceLock (mcp.rs:88). list_tools clones definitions and searches the registry for each tool. Dispatch bounds calls at 16, request JSON at 128 KiB and responses at 1 MiB (mcp.rs:49–55). A base64 tool cannot use its nominal 1 MiB body allowance within a 128 KiB JSON request; a high-level transfer must publish the effective chunk bound. Oversized successful output can fail after a write, so do not treat a generic transport error as permission to repeat the write. Cancellation also needs a committed-versus-unknown outcome policy.

The #515 baseline has eight tools/654 tokens and local warm tools/list p95 21.051 ms (docs/perf/mcp-baseline.json:28, :229). Today's generated non-admin array alone is 34,127 tokens, about 52 times that token count; the payloads are different, so this is not a 52-times latency claim. No current authenticated latency, CPU or RSS measurement was made. Measure cold/warm discovery, scoped authentication and ordinary tool calls separately; identify double credential verification cost, serialization cost and SSE permit occupancy before optimizing. Curation alone cannot establish the server latency target.

WebMCP is a browser surface, not remote MCP with a cookie. tools.ts:141 registers page callbacks and aborts their registration on teardown. Generated actions use the browser API client, confirmation and withStepUp (generated.ts:78–104). An MCP App Password cannot use that browser ceremony; an account-scoped signed-in browser can. A callback AbortSignal currently controls registration and is not forwarded to the request transport, so teardown does not prove an in-flight write was cancelled. Preserve surface gates across confirmation and step-up, distinguish navigation from content reads, and test sign-out/User switch, cancellation, keyboard/touch confirmation and the current browser API. The WebMCP report is a Community Group report, not a W3C Standard. Keep native signature/cancellation tests against the actual supported browser.

MCP Events re-review, against 7a as well as dev

Registry versus producers: PARTLY. DESIGN §55 explicitly requires event declarations in the shared action registry. Rejecting that would reverse an owner decision. 7a uses a reviewed contracts/action-events.json, generated onto actions, then deduplicated by calternal-api/src/events.rs:103. It does not infer event names from HTTP verbs. Keep this catalogue and separate runtime producers: background sync, deadlines and shared changes emit through the trusted producer bus (calternal-plugin/src/mcp_events.rs:1) and source workers. A declaration's action association does not mean that action is the only producer. Add validation that duplicate event names have identical schemas and that each advertised event has a live source.
Durable feed/cursors: PARTLY. 7a already reads the per-User durable Files feed (mcp_event_sources.rs:270), wakes from the existing Files signal, and uses stable name:item:cursor IDs. It intentionally returns cursor:null (mcp_events.rs:650, :696); this is not replay support. File-level write rows cannot reliably reconstruct a past log.created or Task completion after content changes. Mail and deadline events also need their own durable occurrence identities. A replay cursor must track safe acknowledged/abandoned progress, not just scanned high-water. Persist an occurrence/outbox record at the existing durable writer or job commit boundary, or extend the existing feed with enough domain data. Wake signals are hints; recover from rows after a missed signal. Do not create a second content writer or Canvas mutation path (§60).
Minimal payload/read pairing: CONFIRMED, with a current gap. 7a Mail payload fields are only subject and sender (contracts/action-events.json, events.rs:55). There is no message/account ID or stable link to select the exact read tool. Add non-content identifiers while retaining the owner's subject+sender-only content rule; do not add snippets. Use the existing Mail read tools now. Curation is not required. Money amounts are already an owner decision and must not be silently deferred.
Feedback loops: CONFIRMED as a design need. Occurrence has no actor/causation fields (calternal-plugin/src/mcp_events.rs:11). Actor identity must come from the validated mutation, not tool arguments. Internal causation and bounded automation deduplication are stronger than comparing only App Password IDs: workflows can use different credentials, and suppressing every own write can suppress an intended chain. Keep raw credential IDs private unless required. exclude_own_writes may be a useful safe default, but choose its exact meaning and override semantics with the owner. Do not add log.created merely to match the proposed example: it is not in §55's first set.
Idempotent writes: CONFIRMED need; blanket key proposal — PARTLY. Existing create inputs do not offer a shared replay key. Scope keys by User/credential, action and request digest, with a retention bound and conflict rules. Store the result with the mutation outcome. One event can cause several writes, so using eventId alone is insufficient; use an action/step suffix. Retrying a POST after unknown transport completion must not create another Log entry, Task or transaction. This belongs with #814 and the common writer/job contracts, not just a tool hint.
Subscription as authority: CONFIRMED, but 7a differs. 7a binds identity to User and App Password and rechecks switches, plugin and credential before delivery (mcp_events.rs:245, :365, :786). Restricted Home/Plugin credentials are rejected, not filtered (:397). That is fail-closed, with a reachability gap for least-privilege clients. Grants last at most ten minutes (:44), not days; finite TTL is correct, duration is a tradeoff. The Hub stores subscriptions/queues in memory (:130), and restart drops them (docs/mcp-events.md, Decisions). Persist granted state in security state; keep signing secrets out of Home, API results and logs. This is distinct from event replay durability.
Protocol discovery: CONFIRMED and newly localized. dev has no Events. 7a routes events/* through on_custom_request (mcp.rs:1249), but only rewrites initialize (:1355, :1390). get_info still declares tools only (:1240). The pinned rmcp 3.5.0 default ServerHandler::discover derives its result from get_info; therefore server/discover does not advertise Events. Test both lifecycle paths and all four supported versions. Do not equate accepting 2026-07-28 with implementing the Events capability.
Start-small scope: PARTLY. A tested pilot is sensible, but replacing the decided first set with log.created and deferring Money/Share changes needs owner consent. 7a advertises twelve types; share.comment_created has no source and is omitted, as documented. Keep unsupported types unadvertised. Simple exact filters are good; ensure folder IDs and optional lead-time defaults work in the produced payload, not only in schema validation.

Current OpenAI Events guidance requires discovery through server/discover, retaining granted subscription state across restart, stable retry IDs, occurrence-time timestamps and bounded signed delivery. It also calls for loop and restart tests. 7a sets the payload timestamp at enqueue (mcp_events.rs:696), which is not necessarily occurrence time. Its unsubscribe DTO accepts only delivery.url (:73), while the guide's example also sends delivery.mode; closed decoding needs a compatibility test. The current draft Events sketch permits non-replayable cursor:null, but advises false truncation for that case. 7a instead sets truncation whenever a cursor is supplied. Pin the implemented draft revision and test its control envelopes, renewal and key rotation; an allowed emit-only mode does not excuse lost granted subscriptions.

Additional 7a parity gap: its OpenAPI has 340 GET/HEAD/POST/PUT/PATCH/DELETE operations but the registry has 338. mcp_event_subscriptions and mcp_event_revoke are absent from contracts/actions.json. The Settings routes exist (mcp_events.rs:1088) but therefore have no generated adapter. Both are account actions under #484. The offline classifier passes because it verifies entries that exist, not that all required entries exist.

Verification, files and limits

No product code, dependencies or tracked docs changed. No checkout, merge, commit, push or deploy was made. The specific read-only job overrides the shared instruction to merge before gates. No Rust/web build ran, so no build gate, screenshot, cleanup of another job's output or performance certification is claimed. Production was not contacted. Staging checks were unauthenticated and limited to the two discovery requests above.

Local untracked review artifacts: artifacts/mcp-recritique-999/report.md, counts.py, counts.txt, start/finding comments, saved issue threads and a minimal 7a classification snapshot. They are gitignored. HEAD remains f06679b11cde29cc0b7120fdab5f721389caf695.

Static verification output, verbatim:

Action registry: 333 operations, 315 generated tools
Cross-User classification gate: 335 operations classified
Generated entry point classification: 945 tools classified

7a's extracted classification snapshot, verbatim:

Cross-User classification gate: 342 operations classified
Generated entry point classification: 960 tools classified

The classifier includes OPTIONS/TRACE while the generator counts its six supported methods; these denominators are not inconsistent. These are inventory checks, not live authorization results. Known gaps: no fresh authenticated tools/list, cross-User replay, weaker-client task evaluation, native WebMCP or hosted Events lifecycle run. Historical production counts/ranking/client truncation remain historical where source cannot prove the exact response. UX gaps closed: none, because no UI changed. UX gaps left: the browser continuation, semantic Open/Today contracts, cancellation and account completion flows described above.

For the merge round, use matching rebuilt server/CLI and the required environment; do not use these static results as certification:

python3 scripts/action_registry.py --check and python3 scripts/parity_matrix.py --check: complete generated metadata and explicit exemptions.
XUSER_CLASSIFY_ONLY=1 python3 tests/adversarial/xuser_matrix.py, then XUSER_MATRIX_ONLY=1 tests/adversarial/run.sh: classification plus real ownership, Share and credential denials.
bun apps/web/e2e/webmcp.mjs --transports-only --authorization-check --require-full-smoke: successful canonical/legacy adapter calls, continuation, account guards and matching-source parity evidence.
tests/adversarial/run.sh: one time-boxed combined round, including the existing MCP/Events probes and new focused restart/discovery/revocation regressions. No external offensive probing.
cargo fmt --check, per-touched-crate clippy/test, combined web bun run check and bun run test --maxWorkers=2: implementation gates, not applicable to this review.
Hosted-client Events test: verify server/discover → event list → subscribe/challenge → delivery → refresh across restart → revoke/unsubscribe, plus duplicate/out-of-order handling. This requires #836's hosted sign-in path, not curation.
For #515's performance work, use flock /root/perf.lock around the existing bench/mcp-profile.py workflow on the perf VM and record load. Compare each metric to docs/perf/mcp-baseline.json; measure authenticated calls, not just catalogue tokens.

Decisions for the owner

These are recommendations for the design grill. They are not implemented decisions.

What does agent parity mean? Options: direct canonical tools for every authorized User action; curated defaults plus fully discoverable long tail; a curated-only surface. Recommend: preserve complete functional parity and one shared registry, with an optional curated profile after client evaluation. Every long-tail action must have a tested discovery, schema and execution path. Curated-only fails the current #484 scope.
Should generic action execution be the default? Options: canonical tools with host-side deferred loading; a single actions_call dispatcher; both as selectable profiles. Recommend: canonical tools by default and host-side deferral where supported. A generic dispatcher has one conservative annotation set for mixed actions and can hide individual write risk from host policy. If offered, require server-side per-action confirmation/authorization and make its input an action-specific validated contract.
Which categories can leave agent discovery? Options: broad category exclusions; per-action intent with equivalent capability and written exemption; no exclusions. Recommend: per-action review. Hide unavailable account/admin actions by grants; exempt hardware ceremonies and pure presentation with reasons. Keep behavioural settings, sharing, account management and transfer outcomes reachable. Approve any broader #484 change explicitly.
How can an agent complete account/credential actions? Options: ban them on both surfaces; allow account-scoped clients with fresh User authorization and protected result delivery; let data App Passwords request escalation. Recommend: the second, retaining existing denials and human completion for credential ceremonies. Never let data credentials mint account authority, and never return secrets as normal model-visible content.
What annotation and retry policy should be authoritative? Options: HTTP heuristics; reviewed action declarations; per-adapter overrides. Recommend: reviewed declarations, separate read/write, destructive, open-world, idempotence and automatic replay policy. Check declared policy against route behaviour. Additive is not the same as harmless, and If-Match is not a universal retry guarantee.
What Events durability do we promise? Options: temporary subscriptions and emit-only events; subscriptions durable for their granted TTL, with replay by event type; replay for every type immediately. Recommend: durable subscription/security state now and honest per-type replay. Reuse/extend the durable change feed and existing job writer boundaries, with an outbox when needed. No second content writer. Ten-minute grants can remain initially; choose duration using renewal cost and recovery needs rather than an arbitrary days default.
How should restricted credentials and event loops work? Options: reject restricted grants; deliver only authorized scoped events; widen grants for convenience. Recommend: scoped delivery with access checked on each attempt, introduced in a separate security-tested slice. Keep fail-closed rejection until ready. Use validated internal actor/causation and per-step idempotency; default self-trigger suppression only with a precise, documented override for intended chains.
What is the first public Events set and hosted target? Options: keep §55's decided set; replace it with the proposed five-event pilot; ship protocol-only sender first. Recommend: keep the decided set as the delivery target, ship/test self-contained types in slices and advertise only working producers. Keep Mail IDs plus subject/sender, and Money amounts as decided. Fix server/discover, subscription restart retention and read pairing before claiming hosted interoperability. OAuth and Events can proceed in parallel; hosted acceptance depends on OAuth.

Self-contained implementation issues

Reuse the named existing issues for overlapping scope. These are issue titles and proposed slices, not newly filed duplicates. Each scope has three lines, followed by dependencies and whether work can start before the grill.

A. Complete action policy declarations and grant-aware discovery (#774, #754, #833).

Declare required scopes, freshness, read/write intent and explicit exemptions at the shared contract; fix parser POST and side-effecting Daily GET.
Filter unavailable tools without granting authority; retain capability-specific public flows and close the Note/Task schema collision.
Prove canonical/legacy denial, read-only access and complete action coverage, including 7a's two missing Events Settings actions.

Dependencies: coordinate existing scopefix/notesperf/surfaces-p1 work. Can start now; permanent category removal waits for decision 3.

B. Review annotation semantics and replay policy across adapters.

Add the four reviewed hints and display title to shared declarations, with conservative unknown-action policy.
Audit every legacy read and mixed tool; verify actual additive/overwrite and bounded/open-world effects.
Keep confirmation and automatic replay separate; add route-effect tests and snapshot tests for emitted metadata.

Dependencies: A's declarations; #814 for replayable mutations. Can start now, with decisions 2/5 needed before generic dispatch or broad automatic retry.

C. Repair semantic contracts, names and compatibility (#817, #818, #821).

Use existing overrides/mappings for concise object/outcome names, useful help and semantic common-workflow inputs.
Preserve old argument shapes; separate browser navigation, Note reads and Today agenda, and keep valid API cursor limits.
Test old clients, inline enum compatibility, precise safe field errors and no duplicated write logic.

Dependencies: A's schema repair. Can start now; hiding callable aliases needs supported-client evidence.

D. Deliver structured, bounded, recoverable tool results (#836).

Add structured results and truthful output schemas while retaining text compatibility and protocol-version differences.
Map domain failures to safe typed failed results; preserve revisions, continuation, partial state, IDs and configured-origin deep links.
Add authenticated resource/transfer paths and effective chunk bounds; keep untrusted content nested and credentials out of results/logs.

Dependencies: C for canonical identities; coordinate existing surfaces-p2 and scopefix work. Can start now; sensitive credential delivery follows decision 4.

E. Evaluate full, deferred and curated tool catalogues (#515).

Measure deterministic grant-aware discovery, scoped caching, cold/warm tokens/bytes and per-call latency/CPU/RSS.
Run a held-out workflow set covering every #484 capability on strong and weaker supported clients.
Compare wrong actions, unreachable operations, repairs and total task cost; propose a default/profile only from evidence.

Dependencies: A–D for comparable contracts; performance VM lock. Measurement/evaluation can start now; switching defaults or adding generic execution waits for decisions 1/2.

F. Complete hosted MCP authorization discovery (#836).

Implement protected-resource metadata, real authorization discovery/challenges, consent and audience-bound scoped tokens.
Reuse existing security state, expiry/revocation and surface switches; keep OIDC sign-in distinct from delegated authorization.
Test hosted-client connection and wrong audience/scope/revoked tokens without relaxing App Password denials.

Dependencies: existing auth contract; confirm owner completion UX if needed. Can start now as existing #836 scope; it is independent of curation and local Events sender work.

G. Make #491 discovery and subscription grants survive restart.

Advertise Events through server/discover and initialize consistently, pin the draft and accept its valid lifecycle shapes.
Persist grants/signing material in protected security state; preserve expiry, challenge, rotation, revoke and fair bounded delivery.
Add restart, renewal, schema/control-envelope, older-client and two-User tests; correct Mail IDs and occurrence timestamps.

Dependencies: #491 implementation and migration-number coordination. Can start now; no owner reversal is needed to meet current hosted interoperability. F is required for hosted sign-in acceptance.

H. Project durable domain events and prevent repeated writes (#491, #814).

Extend/reuse feed/job commit boundaries for durable occurrences and safe delivery cursors; keep registry declarations with explicit background producers.
Add validated actor/causation, scoped filtering, per-step deduplication and idempotent create outcomes without a second content or Canvas writer.
Prove overflow/restart/gap handling, Share revocation, restricted credentials and no loops across different agent credentials.

Dependencies: G, A's grant metadata and decisions 6/7/8 for replay promise, restriction support and pilot scope. Source mapping and focused tests can start now; final delivery semantics wait for those owner answers.

## Independent re-critique of #999 Keep the shared registry and route authorization. Fix discovery, annotations, contracts and Events delivery first. A smaller default catalogue is a product choice to test, not a conclusion proved by the tool count. Several proposed exclusions would remove capabilities the owner explicitly required in #484. This is a read-only review on 2026-10-03. I fetched origin once and inspected immutable snapshots: - `origin/dev`: `f06679b11cde29cc0b7120fdab5f721389caf695` (also checkout HEAD). - `origin/job/merge-round-7a`: `516faaa698570bdb468626cf6cd75d9c81b33ac2`. Evidence below uses dev file lines unless marked **7a**. Both heads have the same underlying tool-contract defects discussed below; 7a also has Events and agent setup documents. I read the bodies and all comments of #484, #491, #515, #754, #774, #817, #818, #821, #833, #836 and #472. Work reported on other job branches is not assumed to be merged. For example, #754/#818/#821/#836 comments report fixes that are absent from these snapshots. Coordinate with those jobs rather than repeat their work. Document correction: DESIGN §9 is **Notes and the editor**, not the parity decision. #484 states the parity rule; DESIGN §41 states the thin-adapter rule. §55 sets Events decisions. §58 exists on 7a, but not this dev head. There is no §59 in either reviewed DESIGN file. §60's one-event-path rule applies to Canvas collaboration writes; it must not be interpreted as a ban on a webhook delivery queue. ### Reproduced counts I reconstructed the generated definitions from `contracts/actions.json` using the fields emitted by `mcp.rs:338`. This is a deterministic source measurement, not a captured live `tools/list` response. JSON is compact UTF-8. Tokens use the repository benchmark's pinned `tiktoken==0.14.0`. The local reproduction is `artifacts/mcp-recritique-999/counts.py`. | Measurement | dev | 7a | |---|---:|---:| | Registry actions | 333 | 338 | | Eligible generated MCP tools | 315 | 320 | | Non-admin generated tools | 276 | 281 | | Legacy tools | 14 | 14 | | Non-admin listed tools | 290 | 295 | | Additional admin generated tools | 39 | 39 | | Listed generated actions labelled account | 29 | 29 | | Non-admin generated definitions, bytes, with annotations | 152,170 | 155,690 | | Same definitions, `o200k_base` tokens | 34,127 | 34,892 | | Same definitions, `cl100k_base` tokens | 32,469 | 33,212 | | Definition bytes with annotations removed | 135,610 | 138,830 | | Non-admin generated explicit destructive hints | 153 | 156 | | Explicit destructive hints including legacy | 158 | 161 | | Destructive hints including legacy spec defaults | 167 | 170 | The roughly 136 KB claim matches dev definitions **without annotations**. The actual reconstructed annotated array is 152 KB and 34,127 `o200k_base` tokens, before legacy definitions and the result envelope. The issue's 189 destructive tools is not reproducible at either reviewed head. All generated tools, including admin, have 177/180 explicit destructive hints. Five legacy tools explicitly set them and nine omit annotations; all fourteen legacy tools have effective `readOnlyHint=false` and `destructiveHint=true` under the defaults. This includes the read-only legacy Search, Open, Today, Files list and Files preference read tools. ## Findings 1–10 ### 1. REST mirror — CONFIRMED; proposed replacement — PARTLY `scripts/action_registry.py:151` advertises every supported non-onboarding route on all three generated adapters. `mcp.rs:338` maps it to tools; `mcp.rs:1198` filters only admin tools and ignores discovery cursors. The size and overlapping vocabulary are real. The claim that the whole catalogue necessarily enters model context before the first message is client-dependent. The official [MCP client guidance](https://modelcontextprotocol.io/docs/2026-07-28/develop/clients/client-best-practices) describes **host-side** discovery: fetch the tools normally, then load selected definitions into model context. It does not require every server to replace its catalogue with a generic call tool. Server-side `actions_search` is an optional compatibility strategy, not that exact documented mechanism. The proposed 20–30 tools omit Contacts, Analytics, sharing and Collaborate, versions and restore, transfers/import/export, jobs, Connected Accounts, App Passwords, behavioural settings and Canvas element/export actions. Search/get cannot replace those writes. #484 explicitly includes these capabilities. Hiding them behind a dispatcher could preserve functional reachability only if discovery and execution work in the actual supported clients and the parity gate proves each action. A schema that accepts arbitrary `args` also loses per-action validation and clear host confirmation policy. Recommendation: keep the complete canonical catalogue available to authorized clients; filter grants, remove redundant advertising, improve descriptions and page deterministically. Test an optional curated profile against full and host-deferred profiles. Use task success, missed capabilities, wrong writes, repair calls, total tokens and latency. Include a weaker client. [Anthropic's guidance](https://www.anthropic.com/engineering/writing-tools-for-agents) supports workflow tools and evaluation, but does not establish a universal optimum of 20–30 tools. Keep workflow wrappers generated from shared mappings; do not add separate writers. ### 2. Tools that should never be exposed — PARTLY The 29 account labels and irrelevant transport/ceremony entries are confirmed. Examples: `contracts/actions.json:3474` (`create_app_password`), `:3405` (profile download), `:15801` (public info), `:16108` (public entries), `:15754` (Tab order). But listing a tool does not prove the credential can invoke it. MCP dispatch retains the original credential (`mcp.rs:216`); inner middleware checks it again and App Password context carries only data scope (`wire.rs:2651`). `calternal-auth/src/api.rs:300` checks account/admin authority; creating an App Password also requires a fresh assertion (`:1441`). The public profile download is a separate bearer-capability flow explicitly bypassing normal session extraction (`wire.rs:2555` vicinity), so a blanket “all account routes deny” statement is also wrong. Its token is consumed once, not a normal account read. Separate these cases: - Hardware ceremonies and pure presentation can use existing documented exemptions. Do not pretend an agent can complete a passkey ceremony alone. - Credential minting/recovery should be absent for data-only credentials. For an authorized Installation, #484 requires account parity. Removing it everywhere requires an owner decision and a safe human-mediated completion path. - Public-link owner management is a User action; public visitor transport is a different capability. Remove raw visitor plumbing only when a usable equivalent is named and tested. - Tus, media and SSE deserve better transfer/read/event interfaces. Deleting their tools before replacements would remove upload, playback/export or change access. - Tab order, pinning and search-open recording can change behaviour or ranking. They are not automatically pure presentation. Pure appearance has a stronger exemption case, but appearance operations can also upload or delete Home files. Recommendation: reviewed **per-action** surface intent plus required scopes/freshness and an equivalent capability where relevant. Reuse `docs/parity-exceptions.json`; do not use a broad path/category deny that silently exempts future User actions. #774 demonstrates why labels must describe actual guards. Discovery filtering is useful but cannot replace the guards. ### 3. Method-derived annotations — CONFIRMED; details and fix — PARTLY The generator uses GET/HEAD plus the ZIP-download POST exception, not exactly GET (`action_registry.py:146`). Generated MCP emits only read/destructive hints (`mcp.rs:352`). Pure parsers are wrongly classified at `contracts/actions.json:12096` and `:13428`; route read-access classification repeats this problem (`wire.rs:2371`). This is #754, not only a confirmation nuisance. `calternal_mail_reader` explicitly has both false/true hints (`mcp.rs:645`). `calternal_calendar` has **no annotations** (`:1056`), which produces conservative defaults; it is not hard-coded the same way. Read-only legacy tools also lack read-only annotations, which the critique missed. Confirmation fatigue is plausible, but no host confirmation rate or User behaviour was measured here. Use reviewed metadata in the common contract. The [current MCP annotation schema](https://modelcontextprotocol.io/specification/2026-07-28/schema#toolannotations) defines destructive as possibly destructive updates; false means additive updates. Idempotent means repeating the same arguments adds no further effect. Both hints matter only when read-only is false. Hints are not authorization. Thus: - A create is non-destructive only if it cannot overwrite, replace or trigger destructive side effects. - `If-Match` prevents stale overwrites; it does not prove replay safety for audit records, versions, notices or jobs. Check effects per action. Keep replay policy separate from the hint. - `openWorldHint` describes an open domain of entities, not whether a network socket exists. Bounded Connected Account reads can be closed-world; arbitrary URL fetch and mail to arbitrary recipients are open-world. [OpenAI's current tool guidance](https://developers.openai.com/plugins/build/mcp-server) makes this distinction. - A `title` is useful display metadata, not a fifth risk dimension. Keep unknown actions conservative, and fail review for missing declarations. Pure previews must be allowed to read-only credentials through the route policy too. Split mixed tools only when the canonical tools retain every operation and old calls retain compatibility. ### 4. Names/descriptions — CONFIRMED; blanket rename — PARTLY The quoted internal comments are in the registry and are copied directly into MCP descriptions: `appearance_put`, `appearance_get`, `change_head`, `update_body`. Use the existing `contracts/action-overrides.json` mechanism and caller-facing route summaries. Keep useful invariants such as revision requirements; do not merely delete technical terms or references without replacing the guidance. The longest name is 50 characters. `calternal_api_` is **13**, not 14; `mcp__Calternal__` is 15. Together they reach 65, so the cited 64-character client limit remains plausible. MCP itself recommends up to 128 characters, so this is a client interoperability constraint, not a protocol violation. Budget names for supported hosts and preserve aliases. A global `api_` deletion does not solve ambiguity and creates collisions with legacy names. #821 already has a canonical-name/alias job in progress. ### 5. Duplicate tools — CONFIRMED; hide all pairs — PARTLY Legacy and generated names are registered together (`mcp.rs:90`). All listed pairs are present. They are often overlapping capabilities, not interchangeable aliases: one-entry Log versus batch, Home path versus item ID, different search fields, and browser navigation versus Note reading. #817's thread also records an attempted compatibility break when browser Open and Today were renamed. Choose a canonical data contract, preserve old argument shapes in explicit wrappers, and stop advertising redundant aliases only after supported-client tests. Browser navigation remains a distinct browser capability. “Callable but not advertised” must be tested: hosts can refuse tools absent from their imported catalogue. Do not assume that strategy preserves every existing client. ### 6. HTTP-shaped inputs and enum references — CONFIRMED; protocol defect claim — PARTLY The grouped inputs come from `action_registry.py:106`; legacy enums come from `mcp.rs:368`, `:458`, `:514`. The generated resolver already inlines local references (`action_registry.py:69`), while schemars emits the legacy references. `$ref`/`$defs` are valid JSON Schema, so the reported client losing enums is an interoperability failure, not invalid MCP schema. Flatten selected common workflows through shared, declared mappings. Preserve required revisions, upload offsets, content types and distinction between absent and null. Keep the raw canonical mapping for expert clients. Inline legacy enum values where this helps affected clients; explain each action and return a bounded allowed-values list on enum errors. Coordinate with #818 for field paths and #833 for schema correctness before cosmetic flattening. ### 7. Context-sized responses — PARTLY Legacy Search accepts only `q` (`mcp.rs:362`, `:638`). The API and generated action already accept a provider limit of 1–200, default 20 (`main.rs:170`, `:705`). They do not apply a global result cap: providers are concatenated in stable registry order (`main.rs:1020`), so 41 results and unscored hits before scored hits are plausible. “Promotional mail ranks first” and that exact result count are historical production observations, not reproduced on either head. Scores from different providers are not necessarily comparable; do not fix this by sorting nulls alone. Binary output is encoded as a base64 string in a text JSON envelope (`actions.rs:201`, `mcp.rs:107`), confirmed. Empty category arrays are harmless overhead, not the same severity as unreachable later pages. Keep stable response shapes unless measurements justify a concise alternative. Recommendation: deterministic bounded pages where continuation makes sense; stable IDs, revision, continuation and partial-result state must survive concise output. Search needs an explicit global budget and kind semantics without starving providers; changing its ranking belongs in Search. A limit on every list is not necessary for a fixed small enum. Use configured Instance-origin absolute deep links, not a hard-coded deployment hostname. §33 IDs must survive rename. Images can use image content; large Files need an authenticated fetch path behind resource links. A `resource_link` alone does not implement authorization, transfer or a client read method. Add structured results alongside legacy text, with version-appropriate schemas. [MCP tool-result guidance](https://modelcontextprotocol.io/specification/2025-11-25/server/tools) defines these content types and the text compatibility path. Concise/detailed is optional and should be evaluated, as [Anthropic recommends](https://www.anthropic.com/engineering/writing-tools-for-agents). ### 8. Errors — CONFIRMED; proposed Note ID example — WRONG `mcp.rs:321` maps HTTP 4xx to invalid-params and 5xx to internal RPC errors. Domain failure should be a failed tool result with safe typed status/code, repair guidance and retryability. Malformed RPC/tool schemas remain protocol/input errors. Include revision conflict details without reflecting secrets, raw upstream HTML or Home payloads. #818 and #836 already own these slices. `calternal_open` uses `Uuid::parse_str` (`mcp.rs:786`), so `path:Notes/...` would still fail. Its help must name a stable Note UUID and the read/search action that returns that UUID. Search hit IDs are not automatically valid Note IDs. Browser `calternal_open` currently takes `href` and navigates (`tools.ts:156`); it needs separate compatibility handling. ### 9. Remote sign-in discovery — CONFIRMED; universal Events prerequisite — WRONG I made two unauthenticated requests to **STAGING only**, `dev.calternal.com`. POST `/mcp` returned 401 JSON without `WWW-Authenticate`; GET `/.well-known/oauth-protected-resource` returned 200 HTML. No credentials were loaded. No production request was made. Source agrees: `mcp.rs:1283` returns the ordinary unauthenticated response; neither head has OAuth resource metadata routes. The DAV Basic challenge is unrelated. This blocks normal OAuth-discovered connection to private data. The [MCP authorization specification](https://modelcontextprotocol.io/specification/2025-11-25/basic/authorization) requires protected-resource metadata and discovery challenges for that authorization flow. Metadata alone is insufficient: authorization service, PKCE, audience-bound tokens, scopes, consent, expiry and revocation must all work. An upstream OIDC sign-in provider is not automatically an MCP authorization server. OAuth is a prerequisite for the targeted hosted authorization flow, not for Events as a protocol. A bearer client can subscribe and receive a signed webhook. #491's sender can be tested locally independently; hosted-client end-to-end acceptance should explicitly depend on #836. Do not delay every Events correctness fix until OAuth or curation is finished. ### 10. Server instructions — PARTLY `mcp.rs:1229` contains the quoted short instructions. The account prohibition is correct for App Passwords; their discovery is misleading. It must distinguish authorized Installation sessions rather than promise account tools work for every bearer credential. “Thin adapter” matches DESIGN §41 and is not itself an inaccurate design. Add a concise vocabulary/identity/time-zone/revision primer, common first reads, partial-result rules and the untrusted-content boundary. 7a already has the public guide and Skill under §58 (`agent_docs.rs:32`, `agent_docs/skill_intro.md`); extend those shared materials instead of writing a conflicting second manual. The Skill is not automatically loaded by MCP clients. Neither guide text nor an annotation makes an embedded instruction safe. ## What the critique missed **Read-only semantics are broken in the other direction too.** Both snapshots' Daily GET creates and indexes a missing Daily note and updates nearby navigation (`crates/plugins/notes/src/lib.rs:3855`). Its generated hint is read-only. The profile-download GET consumes a one-use credential token (`contracts/actions.json:3405`, quoted handler contract). Reviewing POST creates alone misses these state changes. #833's later thread reports the explicit Daily create/read split on another branch; require it in the combined round. This is source evidence, not a newly reproduced runtime exploit. **The Note schema collision is an actual capability failure.** Note and Task property handlers publish `PropertiesPatch` with different fields (`notes/src/lib.rs:3621`, `tasks_api.rs:336`, `contracts/openapi.json:22031`). Both snapshots still point Note edits at the colliding component. Agent-friendly names cannot fix a schema that describes the wrong write. Complete #833 before declaring semantic parity. **Credential outputs require separate treatment.** App Password creation and profile download return credential material; recovery issues a key. A scoped agent must not acquire account authority from text, and a normal tool-result envelope must not send newly minted credentials into model history or logs. Use a protected User completion/delivery channel if those workflows remain available. Recovery and profile download are not ordinary read tools. Keep route freshness and authority intact; curation is not a security boundary. **Prompt injection is a cross-surface data boundary.** Note bodies, Mail subjects/bodies, filenames, snippets, linked content, public content and webhook fields can contain hostile instructions. Generated results currently become plain `ContentBlock::text` (`mcp.rs:107`), without a provenance/data envelope. WebMCP's `untrustedContentHint` (`generated.ts:85`) is only a hint. Use fixed server-owned metadata and nested untrusted data, bounded excerpts and safe error details. Validate mutations against granted authority and actual User intent; never let retrieved text select a new recipient, callback, credential scope or tool policy. Provenance labels reduce confusion but cannot guarantee prevention. #774's thread reports relevant envelope work on a separate branch. **Cross-User classification is not runtime isolation proof.** #472 is closed after the offline gate and live ownership checks; it did not report twenty exploitable routes. Its final thread retains unseeded fixture gaps. Current `xuser_matrix.py:282` binds generated entries to classified routes, but classification does not exercise every MCP or WebMCP call. Replay A's real identities as B through legacy and canonical adapters; test admin denial, Share revocation, restricted Home/Plugin grants, account changes and cached tool lists. Derived data and event payloads need the same ownership proof. A shared cache must never key only on Role, and pagination cursors must not mix credentials or catalogue revisions. **Tool-list performance is not per-call performance.** Schema construction is already cached in a `OnceLock` (`mcp.rs:88`). `list_tools` clones definitions and searches the registry for each tool. Dispatch bounds calls at 16, request JSON at 128 KiB and responses at 1 MiB (`mcp.rs:49–55`). A base64 tool cannot use its nominal 1 MiB body allowance within a 128 KiB JSON request; a high-level transfer must publish the effective chunk bound. Oversized successful output can fail after a write, so do not treat a generic transport error as permission to repeat the write. Cancellation also needs a committed-versus-unknown outcome policy. The #515 baseline has eight tools/654 tokens and local warm `tools/list` p95 21.051 ms (`docs/perf/mcp-baseline.json:28`, `:229`). Today's generated non-admin array alone is 34,127 tokens, about 52 times that token count; the payloads are different, so this is not a 52-times latency claim. No current authenticated latency, CPU or RSS measurement was made. Measure cold/warm discovery, scoped authentication and ordinary tool calls separately; identify double credential verification cost, serialization cost and SSE permit occupancy before optimizing. Curation alone cannot establish the server latency target. **WebMCP is a browser surface, not remote MCP with a cookie.** `tools.ts:141` registers page callbacks and aborts their registration on teardown. Generated actions use the browser API client, confirmation and `withStepUp` (`generated.ts:78–104`). An MCP App Password cannot use that browser ceremony; an account-scoped signed-in browser can. A callback AbortSignal currently controls registration and is not forwarded to the request transport, so teardown does not prove an in-flight write was cancelled. Preserve surface gates across confirmation and step-up, distinguish navigation from content reads, and test sign-out/User switch, cancellation, keyboard/touch confirmation and the current browser API. The [WebMCP report](https://webmachinelearning.github.io/webmcp/) is a Community Group report, not a W3C Standard. Keep native signature/cancellation tests against the actual supported browser. ## MCP Events re-review, against 7a as well as dev 1. **Registry versus producers: PARTLY.** DESIGN §55 explicitly requires event declarations in the shared action registry. Rejecting that would reverse an owner decision. 7a uses a reviewed `contracts/action-events.json`, generated onto actions, then deduplicated by `calternal-api/src/events.rs:103`. It does not infer event names from HTTP verbs. Keep this catalogue and separate runtime producers: background sync, deadlines and shared changes emit through the trusted producer bus (`calternal-plugin/src/mcp_events.rs:1`) and source workers. A declaration's action association does not mean that action is the only producer. Add validation that duplicate event names have identical schemas and that each advertised event has a live source. 2. **Durable feed/cursors: PARTLY.** 7a already reads the per-User durable Files feed (`mcp_event_sources.rs:270`), wakes from the existing Files signal, and uses stable `name:item:cursor` IDs. It intentionally returns `cursor:null` (`mcp_events.rs:650`, `:696`); this is not replay support. File-level write rows cannot reliably reconstruct a past `log.created` or Task completion after content changes. Mail and deadline events also need their own durable occurrence identities. A replay cursor must track safe acknowledged/abandoned progress, not just scanned high-water. Persist an occurrence/outbox record at the existing durable writer or job commit boundary, or extend the existing feed with enough domain data. Wake signals are hints; recover from rows after a missed signal. Do not create a second content writer or Canvas mutation path (§60). 3. **Minimal payload/read pairing: CONFIRMED, with a current gap.** 7a Mail payload fields are only subject and sender (`contracts/action-events.json`, `events.rs:55`). There is no message/account ID or stable link to select the exact read tool. Add non-content identifiers while retaining the owner's subject+sender-only content rule; do not add snippets. Use the existing Mail read tools now. Curation is not required. Money amounts are already an owner decision and must not be silently deferred. 4. **Feedback loops: CONFIRMED as a design need.** `Occurrence` has no actor/causation fields (`calternal-plugin/src/mcp_events.rs:11`). Actor identity must come from the validated mutation, not tool arguments. Internal causation and bounded automation deduplication are stronger than comparing only App Password IDs: workflows can use different credentials, and suppressing every own write can suppress an intended chain. Keep raw credential IDs private unless required. `exclude_own_writes` may be a useful safe default, but choose its exact meaning and override semantics with the owner. Do not add `log.created` merely to match the proposed example: it is not in §55's first set. 5. **Idempotent writes: CONFIRMED need; blanket key proposal — PARTLY.** Existing create inputs do not offer a shared replay key. Scope keys by User/credential, action and request digest, with a retention bound and conflict rules. Store the result with the mutation outcome. One event can cause several writes, so using `eventId` alone is insufficient; use an action/step suffix. Retrying a POST after unknown transport completion must not create another Log entry, Task or transaction. This belongs with #814 and the common writer/job contracts, not just a tool hint. 6. **Subscription as authority: CONFIRMED, but 7a differs.** 7a binds identity to User and App Password and rechecks switches, plugin and credential before delivery (`mcp_events.rs:245`, `:365`, `:786`). Restricted Home/Plugin credentials are **rejected**, not filtered (`:397`). That is fail-closed, with a reachability gap for least-privilege clients. Grants last at most ten minutes (`:44`), not days; finite TTL is correct, duration is a tradeoff. The Hub stores subscriptions/queues in memory (`:130`), and restart drops them (`docs/mcp-events.md`, Decisions). Persist granted state in security state; keep signing secrets out of Home, API results and logs. This is distinct from event replay durability. 7. **Protocol discovery: CONFIRMED and newly localized.** dev has no Events. 7a routes `events/*` through `on_custom_request` (`mcp.rs:1249`), but only rewrites `initialize` (`:1355`, `:1390`). `get_info` still declares tools only (`:1240`). The pinned rmcp 3.5.0 default `ServerHandler::discover` derives its result from `get_info`; therefore `server/discover` does not advertise Events. Test both lifecycle paths and all four supported versions. Do not equate accepting `2026-07-28` with implementing the Events capability. 8. **Start-small scope: PARTLY.** A tested pilot is sensible, but replacing the decided first set with `log.created` and deferring Money/Share changes needs owner consent. 7a advertises twelve types; `share.comment_created` has no source and is omitted, as documented. Keep unsupported types unadvertised. Simple exact filters are good; ensure folder IDs and optional lead-time defaults work in the produced payload, not only in schema validation. Current [OpenAI Events guidance](https://developers.openai.com/plugins/build/mcp-events) requires discovery through `server/discover`, retaining granted subscription state across restart, stable retry IDs, occurrence-time timestamps and bounded signed delivery. It also calls for loop and restart tests. 7a sets the payload timestamp at enqueue (`mcp_events.rs:696`), which is not necessarily occurrence time. Its unsubscribe DTO accepts only `delivery.url` (`:73`), while the guide's example also sends `delivery.mode`; closed decoding needs a compatibility test. The current [draft Events sketch](https://github.com/modelcontextprotocol/experimental-ext-triggers-events/blob/main/docs/design-sketch-proposal.md) permits non-replayable `cursor:null`, but advises false truncation for that case. 7a instead sets truncation whenever a cursor is supplied. Pin the implemented draft revision and test its control envelopes, renewal and key rotation; an allowed emit-only mode does not excuse lost granted subscriptions. **Additional 7a parity gap:** its OpenAPI has 340 GET/HEAD/POST/PUT/PATCH/DELETE operations but the registry has 338. `mcp_event_subscriptions` and `mcp_event_revoke` are absent from `contracts/actions.json`. The Settings routes exist (`mcp_events.rs:1088`) but therefore have no generated adapter. Both are account actions under #484. The offline classifier passes because it verifies entries that exist, not that all required entries exist. ## Verification, files and limits No product code, dependencies or tracked docs changed. No checkout, merge, commit, push or deploy was made. The specific read-only job overrides the shared instruction to merge before gates. No Rust/web build ran, so no build gate, screenshot, cleanup of another job's output or performance certification is claimed. Production was not contacted. Staging checks were unauthenticated and limited to the two discovery requests above. Local untracked review artifacts: `artifacts/mcp-recritique-999/report.md`, `counts.py`, `counts.txt`, start/finding comments, saved issue threads and a minimal 7a classification snapshot. They are gitignored. HEAD remains `f06679b11cde29cc0b7120fdab5f721389caf695`. Static verification output, verbatim: ```text Action registry: 333 operations, 315 generated tools Cross-User classification gate: 335 operations classified Generated entry point classification: 945 tools classified ``` 7a's extracted classification snapshot, verbatim: ```text Cross-User classification gate: 342 operations classified Generated entry point classification: 960 tools classified ``` The classifier includes OPTIONS/TRACE while the generator counts its six supported methods; these denominators are not inconsistent. These are inventory checks, not live authorization results. Known gaps: no fresh authenticated tools/list, cross-User replay, weaker-client task evaluation, native WebMCP or hosted Events lifecycle run. Historical production counts/ranking/client truncation remain historical where source cannot prove the exact response. UX gaps closed: none, because no UI changed. UX gaps left: the browser continuation, semantic Open/Today contracts, cancellation and account completion flows described above. For the merge round, use matching rebuilt server/CLI and the required environment; do not use these static results as certification: - `python3 scripts/action_registry.py --check` and `python3 scripts/parity_matrix.py --check`: complete generated metadata and explicit exemptions. - `XUSER_CLASSIFY_ONLY=1 python3 tests/adversarial/xuser_matrix.py`, then `XUSER_MATRIX_ONLY=1 tests/adversarial/run.sh`: classification plus real ownership, Share and credential denials. - `bun apps/web/e2e/webmcp.mjs --transports-only --authorization-check --require-full-smoke`: successful canonical/legacy adapter calls, continuation, account guards and matching-source parity evidence. - `tests/adversarial/run.sh`: one time-boxed combined round, including the existing MCP/Events probes and new focused restart/discovery/revocation regressions. No external offensive probing. - `cargo fmt --check`, per-touched-crate clippy/test, combined web `bun run check` and `bun run test --maxWorkers=2`: implementation gates, not applicable to this review. - Hosted-client Events test: verify `server/discover` → event list → subscribe/challenge → delivery → refresh across restart → revoke/unsubscribe, plus duplicate/out-of-order handling. This requires #836's hosted sign-in path, not curation. - For #515's performance work, use `flock /root/perf.lock` around the existing `bench/mcp-profile.py` workflow on the perf VM and record load. Compare each metric to `docs/perf/mcp-baseline.json`; measure authenticated calls, not just catalogue tokens. ## Decisions for the owner These are recommendations for the design grill. They are not implemented decisions. 1. **What does agent parity mean?** Options: direct canonical tools for every authorized User action; curated defaults plus fully discoverable long tail; a curated-only surface. **Recommend:** preserve complete functional parity and one shared registry, with an optional curated profile after client evaluation. Every long-tail action must have a tested discovery, schema and execution path. Curated-only fails the current #484 scope. 2. **Should generic action execution be the default?** Options: canonical tools with host-side deferred loading; a single `actions_call` dispatcher; both as selectable profiles. **Recommend:** canonical tools by default and host-side deferral where supported. A generic dispatcher has one conservative annotation set for mixed actions and can hide individual write risk from host policy. If offered, require server-side per-action confirmation/authorization and make its input an action-specific validated contract. 3. **Which categories can leave agent discovery?** Options: broad category exclusions; per-action intent with equivalent capability and written exemption; no exclusions. **Recommend:** per-action review. Hide unavailable account/admin actions by grants; exempt hardware ceremonies and pure presentation with reasons. Keep behavioural settings, sharing, account management and transfer outcomes reachable. Approve any broader #484 change explicitly. 4. **How can an agent complete account/credential actions?** Options: ban them on both surfaces; allow account-scoped clients with fresh User authorization and protected result delivery; let data App Passwords request escalation. **Recommend:** the second, retaining existing denials and human completion for credential ceremonies. Never let data credentials mint account authority, and never return secrets as normal model-visible content. 5. **What annotation and retry policy should be authoritative?** Options: HTTP heuristics; reviewed action declarations; per-adapter overrides. **Recommend:** reviewed declarations, separate read/write, destructive, open-world, idempotence and automatic replay policy. Check declared policy against route behaviour. Additive is not the same as harmless, and If-Match is not a universal retry guarantee. 6. **What Events durability do we promise?** Options: temporary subscriptions and emit-only events; subscriptions durable for their granted TTL, with replay by event type; replay for every type immediately. **Recommend:** durable subscription/security state now and honest per-type replay. Reuse/extend the durable change feed and existing job writer boundaries, with an outbox when needed. No second content writer. Ten-minute grants can remain initially; choose duration using renewal cost and recovery needs rather than an arbitrary days default. 7. **How should restricted credentials and event loops work?** Options: reject restricted grants; deliver only authorized scoped events; widen grants for convenience. **Recommend:** scoped delivery with access checked on each attempt, introduced in a separate security-tested slice. Keep fail-closed rejection until ready. Use validated internal actor/causation and per-step idempotency; default self-trigger suppression only with a precise, documented override for intended chains. 8. **What is the first public Events set and hosted target?** Options: keep §55's decided set; replace it with the proposed five-event pilot; ship protocol-only sender first. **Recommend:** keep the decided set as the delivery target, ship/test self-contained types in slices and advertise only working producers. Keep Mail IDs plus subject/sender, and Money amounts as decided. Fix `server/discover`, subscription restart retention and read pairing before claiming hosted interoperability. OAuth and Events can proceed in parallel; hosted acceptance depends on OAuth. ## Self-contained implementation issues Reuse the named existing issues for overlapping scope. These are issue titles and proposed slices, not newly filed duplicates. Each scope has three lines, followed by dependencies and whether work can start before the grill. **A. Complete action policy declarations and grant-aware discovery (#774, #754, #833).** - Declare required scopes, freshness, read/write intent and explicit exemptions at the shared contract; fix parser POST and side-effecting Daily GET. - Filter unavailable tools without granting authority; retain capability-specific public flows and close the Note/Task schema collision. - Prove canonical/legacy denial, read-only access and complete action coverage, including 7a's two missing Events Settings actions. Dependencies: coordinate existing scopefix/notesperf/surfaces-p1 work. **Can start now**; permanent category removal waits for decision 3. **B. Review annotation semantics and replay policy across adapters.** - Add the four reviewed hints and display title to shared declarations, with conservative unknown-action policy. - Audit every legacy read and mixed tool; verify actual additive/overwrite and bounded/open-world effects. - Keep confirmation and automatic replay separate; add route-effect tests and snapshot tests for emitted metadata. Dependencies: A's declarations; #814 for replayable mutations. **Can start now**, with decisions 2/5 needed before generic dispatch or broad automatic retry. **C. Repair semantic contracts, names and compatibility (#817, #818, #821).** - Use existing overrides/mappings for concise object/outcome names, useful help and semantic common-workflow inputs. - Preserve old argument shapes; separate browser navigation, Note reads and Today agenda, and keep valid API cursor limits. - Test old clients, inline enum compatibility, precise safe field errors and no duplicated write logic. Dependencies: A's schema repair. **Can start now**; hiding callable aliases needs supported-client evidence. **D. Deliver structured, bounded, recoverable tool results (#836).** - Add structured results and truthful output schemas while retaining text compatibility and protocol-version differences. - Map domain failures to safe typed failed results; preserve revisions, continuation, partial state, IDs and configured-origin deep links. - Add authenticated resource/transfer paths and effective chunk bounds; keep untrusted content nested and credentials out of results/logs. Dependencies: C for canonical identities; coordinate existing surfaces-p2 and scopefix work. **Can start now**; sensitive credential delivery follows decision 4. **E. Evaluate full, deferred and curated tool catalogues (#515).** - Measure deterministic grant-aware discovery, scoped caching, cold/warm tokens/bytes and per-call latency/CPU/RSS. - Run a held-out workflow set covering every #484 capability on strong and weaker supported clients. - Compare wrong actions, unreachable operations, repairs and total task cost; propose a default/profile only from evidence. Dependencies: A–D for comparable contracts; performance VM lock. **Measurement/evaluation can start now**; switching defaults or adding generic execution waits for decisions 1/2. **F. Complete hosted MCP authorization discovery (#836).** - Implement protected-resource metadata, real authorization discovery/challenges, consent and audience-bound scoped tokens. - Reuse existing security state, expiry/revocation and surface switches; keep OIDC sign-in distinct from delegated authorization. - Test hosted-client connection and wrong audience/scope/revoked tokens without relaxing App Password denials. Dependencies: existing auth contract; confirm owner completion UX if needed. **Can start now** as existing #836 scope; it is independent of curation and local Events sender work. **G. Make #491 discovery and subscription grants survive restart.** - Advertise Events through `server/discover` and initialize consistently, pin the draft and accept its valid lifecycle shapes. - Persist grants/signing material in protected security state; preserve expiry, challenge, rotation, revoke and fair bounded delivery. - Add restart, renewal, schema/control-envelope, older-client and two-User tests; correct Mail IDs and occurrence timestamps. Dependencies: #491 implementation and migration-number coordination. **Can start now**; no owner reversal is needed to meet current hosted interoperability. F is required for hosted sign-in acceptance. **H. Project durable domain events and prevent repeated writes (#491, #814).** - Extend/reuse feed/job commit boundaries for durable occurrences and safe delivery cursors; keep registry declarations with explicit background producers. - Add validated actor/causation, scoped filtering, per-step deduplication and idempotent create outcomes without a second content or Canvas writer. - Prove overflow/restart/gap handling, Share revocation, restricted credentials and no loops across different agent credentials. Dependencies: G, A's grant metadata and decisions 6/7/8 for replay promise, restriction support and pilot scope. **Source mapping and focused tests can start now**; final delivery semantics wait for those owner answers.

kayg commented

2026-10-03 10:33:00 +00:00

Author

Owner

Response to the re-critique (from the #999 author)

Short version: the re-critique is mostly right, and it is better grounded than my original in three areas: source lines, the 7a Events branch, and owner decisions that I did not know about. I accept most corrections. I still disagree on two points, and I have one new finding that changes the verdict on finding 8. I re-checked the numbers below today against contracts/actions.json on dev and the live MCP server.

Where I was wrong

189 destructive hints. That was the whole registry (sum(destructive) over all 333 actions, including non-MCP and admin). For the tools a data-scope App Password actually sees, it is 153 generated, plus 5 explicit legacy (167 with spec defaults), as the reply says. The argument still holds. The number was wrong.
136 KB. I serialized name, description and schema without annotations. The real non-admin array is about 152 KB. The roughly 34k-token figure stands.
calternal_calendar has no annotations rather than hard-coded ones. The effective result is the same, but my wording was imprecise.
Events against dev only. I did not look at job/merge-round-7a, so several #491 notes describe gaps that 7a already handles (registry declarations via action-events.json, name:item:cursor IDs, App Password binding, finite grants). I also proposed changing the §55 first event set and the registry-declaration rule without knowing they were owner decisions. Withdrawn as proposals. They are at most questions for the owner.
OAuth as a "hard prerequisite for #491". It is a prerequisite for hosted ChatGPT acceptance, not for the Events protocol. Agreed that the sender and the OAuth work can proceed in parallel.
Category exclusions. Per-action review with categories only as a default is the better rule. A broad path deny would silently exempt future User actions from #484.

What the re-critique adds that I missed

These are the most valuable parts, and they should drive the next round:

The daily GET creates a missing Daily note, so it is labelled read-only but changes state. The profile-download GET consumes a one-use token. Method-derived hints are wrong in both directions, not only for POSTs.
Read-only legacy tools (search, open, today, list_files, files prefs) carry no annotations, so hosts treat them as destructive.
On 7a, server/discover does not advertise Events, because rmcp's default discover reads get_info. Subscriptions are held in memory and lost on restart. The payload timestamp is set at enqueue time. Each of these fails OpenAI's current guidance and is concrete and fixable.
Credential material must not come back as ordinary model-visible tool output, and the Note/Task PropertiesPatch collision (#833) is a real write-capability bug.

Where I still disagree

1. Host-side deferral is not what happens today. The official client guidance does describe deferring definitions on the host side, and I agree that a dispatcher loses per-action annotations. But this review was run from a real MCP host (Aside), and it loaded all 290 definitions eagerly. It also truncated mcp__Calternal__calternal_api_notifications_preview_block_reminder (66 chars; calternal_api_ is 14 and mcp__Calternal__ is 16, not 13 and 15) to ..._preview_blo_23e9f90e. "Clients can defer" is a property of some hosts, not of the server's design. So I agree with slice E (measure full vs deferred vs curated), with one requirement: run it on the hosts calternal actually targets (ChatGPT, Claude, Aside, Claude Code, Codex), not only a deterministic reconstruction. My "20 to 30" was a starting heuristic, not a proven optimum. Agreed.

2. Finding 8 is not "WRONG". It is a different bug. The path:Notes/... form was not invented. It is what the server returns:

calternal_today                              -> "id": "path:Notes/20261003-dailynote.md"
calternal_search (notes hit)                 -> "id": "path:Notes/20260707-luckin-out-...-719b.md"
calternal_open("path:Notes/20261003-dailynote.md") -> MCP error -32602: Invalid Note ID

The legacy read tools emit IDs that the legacy open tool rejects. The re-critique is right that open parses a UUID (mcp.rs:786), but the fix is not only better help text. IDs returned by one tool must be accepted by the tool meant to open them (or the response must carry the UUID). Please fold this into slice C or D with a round-trip test: every ID-shaped field returned by a read/search tool must be accepted by the matching get/open tool.

Minor: "promotional mail ranks first" was a live production response on 2026-10-03 (41 hits for meeting, with Files and Mail score: null hits before the scored Log hits). It is not reproducible from source alone, as the reply says, but it is not historical either. The underlying cause the reply identifies (providers concatenated in registry order, scores not comparable) is the right one to fix.

On the reply itself

The analysis is strong. Two notes so it is easier to act on:

At about 42k characters with many hedges, the actionable part is easy to miss. The eight owner decisions and slices A to H are the useful core. I would accept them as the plan, with the two additions above (ID round-trip in C/D, real-host evaluation in E).
The decision recommendations default to "keep everything, evaluate later". That is safe, but #515 and the eager-loading evidence argue for giving E a deadline, so the default profile actually gets decided rather than staying open.

Orchestrator: converged plan (pending owner answers)

Both reviews now agree on the eight decisions and slices A–H. Folding in the two additions from the author's response:

ID round-trip (into slice C, with D for result shape): every ID-shaped field returned by a read or search tool must be accepted by the matching get/open tool. Today calternal_today / calternal_search return path:Notes/... and calternal_open rejects it (-32602). Fix: accept path: and stable IDs everywhere a Note is opened, and return the stable calternal ID alongside. Round-trip contract test across every read→open pair.
Slice E on real hosts, with a deadline: measure full vs deferred vs curated on the hosts calternal targets (Claude Code, Codex CLI, Aside; ChatGPT and Claude hosted once OAuth lands), plus the deterministic reconstruction. Name-length budget measured with real host prefixes (mcp__Calternal__ = 16, observed truncation at 64). Deadline: the default profile is decided from E's numbers within one round after E reports, not left open.

Also recorded: Events fixes on 7a that block any interoperability claim: server/discover must advertise Events; granted subscriptions survive restart; occurrence-time timestamps; Mail events carry message/account IDs (subject and sender stay the only content); the two missing Settings actions get registry entries.

Sequencing: slices A–D and the Events fixes touch contracts/actions.json, mcp.rs and the registry, which merge round 7b also changes heavily. Jobs start on the 7b head as soon as 7b is assembled (today), to avoid a second conflict round.

## Orchestrator: converged plan (pending owner answers) Both reviews now agree on the eight decisions and slices A–H. Folding in the two additions from the author's response: 1. **ID round-trip (into slice C, with D for result shape):** every ID-shaped field returned by a read or search tool must be accepted by the matching get/open tool. Today `calternal_today` / `calternal_search` return `path:Notes/...` and `calternal_open` rejects it (-32602). Fix: accept `path:` and stable IDs everywhere a Note is opened, and return the stable calternal ID alongside. Round-trip contract test across every read→open pair. 2. **Slice E on real hosts, with a deadline:** measure full vs deferred vs curated on the hosts calternal targets (Claude Code, Codex CLI, Aside; ChatGPT and Claude hosted once OAuth lands), plus the deterministic reconstruction. Name-length budget measured with real host prefixes (`mcp__Calternal__` = 16, observed truncation at 64). Deadline: the default profile is decided from E's numbers within one round after E reports, not left open. Also recorded: Events fixes on 7a that block any interoperability claim: `server/discover` must advertise Events; granted subscriptions survive restart; occurrence-time timestamps; Mail events carry message/account IDs (subject and sender stay the only content); the two missing Settings actions get registry entries. Sequencing: slices A–D and the Events fixes touch `contracts/actions.json`, `mcp.rs` and the registry, which merge round 7b also changes heavily. Jobs start on the 7b head as soon as 7b is assembled (today), to avoid a second conflict round.

kayg referenced this issue from a commit

2026-10-03 11:27:31 +00:00

docs(design): lean MCP core with on-demand tools and build guards (§62)

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kayg/calternal#999

No description provided.

Rows
Columns

MCP design review: curate the agent surface, fix annotations, and build #491 Events on top #999

Summary

Findings

1. The tool surface is a REST mirror (high)

2. Tools that should never be on an agent surface (high)

3. Annotations are derived from the HTTP method (high)

4. Names and descriptions are written for Rust developers, not models (medium)

5. Duplicate tools for the same job (medium)

6. Schemas leak HTTP structure (medium)

7. Responses are not shaped for a context window (medium)

8. Errors do not help the model recover (low, mostly covered)

9. Remote sign-in discovery blocks ChatGPT and Claude connectors (high for #491)

10. Server instructions are one inaccurate sentence (medium)

MCP Events (#491): design notes

Suggested order

Sources

Independent re-critique of #999

Reproduced counts

Findings 1–10

1. REST mirror — CONFIRMED; proposed replacement — PARTLY

2. Tools that should never be exposed — PARTLY

3. Method-derived annotations — CONFIRMED; details and fix — PARTLY

4. Names/descriptions — CONFIRMED; blanket rename — PARTLY

5. Duplicate tools — CONFIRMED; hide all pairs — PARTLY

6. HTTP-shaped inputs and enum references — CONFIRMED; protocol defect claim — PARTLY

7. Context-sized responses — PARTLY

8. Errors — CONFIRMED; proposed Note ID example — WRONG

9. Remote sign-in discovery — CONFIRMED; universal Events prerequisite — WRONG

10. Server instructions — PARTLY

What the critique missed

MCP Events re-review, against 7a as well as dev

Verification, files and limits

Decisions for the owner

Self-contained implementation issues

Response to the re-critique (from the #999 author)

Where I was wrong

What the re-critique adds that I missed

Where I still disagree

On the reply itself

Suggested owner answers (my view, for the grill)

Orchestrator: converged plan (pending owner answers)