diff --git a/ROADMAP.md b/ROADMAP.md index bd1519de0..72d9f4483 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1179,3 +1179,4 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [119-scan-default-trust](./specs/119-scan-default-trust/) | — | — | | [120-scan-structural-gate](./specs/120-scan-structural-gate/) | — | — | | [122-scanner-skillspector-checks](./specs/122-scanner-skillspector-checks/) | `shipped` | 31/31 (100%) | +| [123-day-two-activation](./specs/123-day-two-activation/) | `drafted` | 0/78 (0%) | diff --git a/specs/123-day-two-activation/checklists/requirements.md b/specs/123-day-two-activation/checklists/requirements.md new file mode 100644 index 000000000..8c83144b0 --- /dev/null +++ b/specs/123-day-two-activation/checklists/requirements.md @@ -0,0 +1,34 @@ +# Specification Quality Checklist: Day-Two Activation (E3) + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-10-20 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) — code references are context only (house style); requirements state outcomes +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- House style (Specs 108-117) keeps file:line context tables in the spec; FRs stay outcome-level. Accepted deviation from the template's "no implementation details" rule. diff --git a/specs/123-day-two-activation/contracts/activity-digest.openapi.yaml b/specs/123-day-two-activation/contracts/activity-digest.openapi.yaml new file mode 100644 index 000000000..a9512ccdc --- /dev/null +++ b/specs/123-day-two-activation/contracts/activity-digest.openapi.yaml @@ -0,0 +1,72 @@ +openapi: 3.0.3 +info: + title: MCPProxy Spec 123 - activity summary digest (additive) + version: "123" +paths: + /api/v1/activity/summary: + get: + summary: Existing endpoint. Spec 123 adds `view` (telemetry view counting, FR-032) and the `digest` response section (FR-018). Existing params and fields unchanged. + parameters: + - { name: period, in: query, schema: { type: string, enum: ["1h", "24h", "7d", "30d"] } } + - { name: view, in: query, required: false, schema: { type: string, enum: [web, tray, cli] }, description: "Unknown values ignored (no error). Counted at most once per surface per hour." } + responses: + "200": + description: Summary (existing fields + digest) + content: + application/json: + schema: + type: object + description: Standard envelope; existing summary fields and the new digest live under data. + properties: + success: { type: boolean } + data: + type: object + properties: + digest: { $ref: "#/components/schemas/ActivityDigest" } +components: + schemas: + ActivityDigest: + type: object + properties: + calls_brokered: + type: object + properties: + total: { type: integer } + by_client: { type: array, maxItems: 3, items: { type: object, properties: { client: {type: string}, count: {type: integer} } } } + other: { type: integer } + unknown: { type: integer } + coverage: { type: string, enum: [complete, partial], description: "partial while legacy tool_call records without the dispatched key are in the window" } + held_for_review: + type: object + description: "null figures + reason=not_attributable when profile/client/token filter is set (FR-023). Scoped callers: only allowed servers." + properties: + entered: { type: integer, nullable: true } + new: { type: integer, nullable: true } + changed: { type: integer, nullable: true } + currently_awaiting: { type: integer, nullable: true } + coverage: { type: string, enum: [complete, partial] } + reason: { type: string, nullable: true, enum: [not_attributable] } + blocked: + type: object + properties: + total: { type: integer } + quarantine: { type: integer } + profile: { type: integer } + other: { type: integer } + responses_blocked: { type: integer, description: "post-dispatch output blocks (output_sanitisation, output_schema); not in total; those calls are brokered" } + sensitive_data: + type: object + properties: { calls_with_detections: { type: integer } } + coverage: + type: object + properties: + state: { type: string, enum: [complete, partial, unavailable], description: "unavailable for scoped callers (watermark is installation-wide)" } + complete_since: { type: string, format: date-time, nullable: true } + window: + type: object + description: "Shared source for the 'this week' vs 'since (N days)' label. effective_start = max(start, install anchor); anchorless installs always label=period." + properties: + start: { type: string, format: date-time } + effective_start: { type: string, format: date-time } + label: { type: string, enum: [period, since_install] } + days: { type: integer } diff --git a/specs/123-day-two-activation/contracts/onboarding-starter.openapi.yaml b/specs/123-day-two-activation/contracts/onboarding-starter.openapi.yaml new file mode 100644 index 000000000..13a509c8c --- /dev/null +++ b/specs/123-day-two-activation/contracts/onboarding-starter.openapi.yaml @@ -0,0 +1,102 @@ +openapi: 3.0.3 +info: + title: MCPProxy Spec 123 - starter offer and onboarding marks (additive) + version: "123" + description: All responses use the standard envelope {success, data} (internal/httpapi/server.go writeSuccess); schemas below describe `data`. Errors are {success:false, error, code}. +paths: + /api/v1/onboarding/starter: + get: + summary: Starter offer decision (FR-012). Gated by requireAdminRead. Personal edition; server edition returns eligible=false reason=server_edition. + responses: + "200": + description: Offer + content: + application/json: + schema: + type: object + properties: + success: { type: boolean } + data: { $ref: "#/components/schemas/StarterOffer" } + post: + summary: Add the selected starter through the catalog door's add step, using ONE resolved entry snapshot that was validated against shown_command (FR-003, research R3). Gated by requireServerOp(ServerOpAddFromRegistry), exactly like the catalog door; agent tokens get 403 with no side effects. Never approves. + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [kind, surface, shown_command] + properties: + kind: { type: string, enum: [time, sequentialthinking] } + surface: { type: string, enum: [wizard, home_web, home_native] } + shown_command: { type: string, description: "Exact command the user saw; must equal the add-time resolution." } + responses: + "200": + description: Added (quarantined unless quarantine is globally off) + content: + application/json: + schema: + type: object + properties: + success: { type: boolean } + data: + type: object + properties: + server: { type: object, properties: { name: {type: string}, quarantined: {type: boolean}, incarnation: {type: integer} } } + offer: { $ref: "#/components/schemas/StarterOffer" } + "409": + description: "code=starter_changed (resolved command differs from shown_command; nothing added; body carries fresh offer) or code=not_eligible" + "400": + description: "code=duplicate_name (passed through from the catalog door) or invalid kind/surface" + "403": + description: "agent token / insufficient server-op permission; nothing resolved, added or recorded" + /api/v1/onboarding/mark: + post: + summary: Existing endpoint; Spec 123 adds optional sticky booleans and a failure enum. + requestBody: + content: + application/json: + schema: + type: object + properties: + starter_offered: { type: boolean, enum: [true] } + starter_declined: { type: boolean, enum: [true] } + starter_failure: { type: string, enum: [connect_failed, capture_timeout] } + second_client_offered: { type: boolean, enum: [true] } + second_client_none_detected: { type: boolean, enum: [true] } + second_client_skipped: { type: boolean, enum: [true] } + /api/v1/onboarding/state: + get: + summary: Existing endpoint; response gains `day_two` (local facts, same shape as data-model 4c minus summary_views) and `detected_unconnected_clients` (registry ids, FR-013). +components: + schemas: + StarterOffer: + type: object + required: [eligible, reason, preselect, facts] + properties: + eligible: { type: boolean } + reason: { type: string, enum: [eligible, launcher_missing, name_taken, reference_overridden, tools_present, held_tools, already_configured, server_edition] } + preselect: { type: boolean } + launcher: { type: string, enum: [uvx, npx_only, none] } + candidate: + nullable: true + type: object + properties: + kind: { type: string, enum: [time, sequentialthinking] } + catalog_id: { type: string } + command: { type: string } + isolation: { type: string, enum: [host, docker] } + expected_tools: { type: array, items: { type: string } } + first_prompt: { type: string } + starter_server: + nullable: true + type: object + properties: { name: {type: string}, quarantined: {type: boolean} } + facts: + type: object + properties: + offered: { type: boolean } + added: { type: boolean } + live: { type: boolean } + declined: { type: boolean } + last_failure: { type: string, enum: ["", connect_failed, capture_timeout, approval_abandoned] } diff --git a/specs/123-day-two-activation/contracts/telemetry-day-two.schema.json b/specs/123-day-two-activation/contracts/telemetry-day-two.schema.json new file mode 100644 index 000000000..ec655da02 --- /dev/null +++ b/specs/123-day-two-activation/contracts/telemetry-day-two.schema.json @@ -0,0 +1,46 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "mcpproxy/telemetry/schema-17/day_two", + "title": "Heartbeat schema 17 additions (Spec 123)", + "type": "object", + "properties": { + "activation_additions": { + "type": "object", + "additionalProperties": false, + "properties": { + "first_tool_indexed_ever": { "type": "boolean" }, + "first_tool_indexed_after": { "enum": ["", "lt_10m", "10m_1h", "1h_24h", "1d_7d", "gt_7d"] } + } + }, + "day_two": { + "type": "object", + "additionalProperties": false, + "properties": { + "starter_eligibility": { "enum": ["", "eligible", "launcher_missing", "name_taken", "reference_overridden", "already_configured", "tools_present", "held_tools", "not_fresh"] }, + "starter_launcher": { "enum": ["", "uvx", "npx_only", "none"] }, + "starter_offered": { "type": "boolean" }, + "starter_added": { "type": "boolean" }, + "starter_live": { "type": "boolean" }, + "starter_declined": { "type": "boolean" }, + "starter_last_failure": { "enum": ["", "connect_failed", "capture_timeout", "approval_abandoned"] }, + "starter_kind": { "enum": ["", "time", "sequentialthinking"] }, + "starter_surface": { "enum": ["", "wizard", "home_web", "home_native"] }, + "second_client_offered": { "type": "boolean" }, + "second_client_none_detected": { "type": "boolean" }, + "second_client_skipped": { "type": "boolean" }, + "second_client_connected": { "type": "boolean" }, + "clients_day0": { "enum": ["", "0", "1", "2", "3_plus"] }, + "clients_day0_final": { "type": "boolean" }, + "summary_views": { + "type": "object", + "additionalProperties": false, + "properties": { + "web": { "enum": ["0", "1", "2_5", "6_plus"] }, + "tray": { "enum": ["0", "1", "2_5", "6_plus"] }, + "cli": { "enum": ["0", "1", "2_5", "6_plus"] } + } + } + } + } + } +} diff --git a/specs/123-day-two-activation/data-model.md b/specs/123-day-two-activation/data-model.md new file mode 100644 index 000000000..773824498 --- /dev/null +++ b/specs/123-day-two-activation/data-model.md @@ -0,0 +1,167 @@ +# Data Model: Day-Two Activation (Spec 123) + +All persisted additions are additive and `omitempty`. An older binary ignores them (downgrade-safe), and a newer binary treats their absence as "unknown" or "false". + +## 1. `storage.OnboardingState` additions (bbolt bucket `onboarding`, key `wizard_state`) + +Base struct: `internal/storage/models.go:102`. + +| Field | JSON | Type | Written by | Rules | +|---|---|---|---|---| +| `InstallFirstRunAt` | `install_first_run_at` | `time.Time` | `EnsureInstallAnchor` at startup, before any other writer; also the missing-record branch of `UpdateOnboardingState` as a safeguard (research R5) | Set once and never changed. Zero means an upgraded install with no anchor; it is never back-filled. | +| `StarterEligibility` | `starter_eligibility` | `string` enum | the first starter eligibility evaluation on a fresh install | Set once. Values: `eligible`, `launcher_missing`, `name_taken`, `reference_overridden`, `already_configured`, `tools_present`, `held_tools`, `not_fresh`. | +| `StarterLauncher` | `starter_launcher` | `string` enum | every fresh-install evaluation | Values: `uvx`, `npx_only`, `none`. The first value is kept for telemetry (sticky). The live value is recomputed per request and not stored. | +| `StarterOffered` | `starter_offered` | `bool` | mark endpoint (`starter_offered: true`) | Sticky. Only false → true. | +| `StarterAdded` | `starter_added` | `bool` | `POST /onboarding/starter` success | Sticky. | +| `StarterLive` | `starter_live` | `bool` | facts loop from commit-time index evidence (research R4) | Sticky. Set only from index evidence for the recorded `(StarterServerName, StarterServerIncarnation)`, compared inside the update transaction. | +| `StarterDeclined` | `starter_declined` | `bool` | mark endpoint | Sticky. It stops pre-selection only. | +| `StarterLastFailure` | `starter_last_failure` | `string` enum | mark endpoint (`capture_timeout`, `connect_failed`) or core (`approval_abandoned`) | Overwritable, latest value. It never changes `StarterLive`. | +| `StarterKind` | `starter_kind` | `string` enum | first successful add | Set once. Values: `time`, `sequentialthinking`. | +| `StarterSurface` | `starter_surface` | `string` enum | first successful add | Set once. Values: `wizard`, `home_web`, `home_native`. | +| `StarterAddedAt` | `starter_added_at` | `time.Time` | first successful add | Set once. Local only, never sent. | +| `StarterServerName` | `starter_server_name` | `string` | each successful add | Local only, never sent. | +| `StarterServerIncarnation` | `starter_server_incarnation` | `int64` | each successful add, from the create commit's `CommitResult` (never a post-add lookup) | Local only. It binds the live and abandonment checks to the added server, so a same-name replacement never counts. | +| `StarterCapturedAt` | `starter_captured_at` | `time.Time` | facts loop from capture-completion evidence (research R4), at its commit time | Set once per starter incarnation. Local only. The abandonment clock (R4a). `definitions_captured` is **not** used, because it is true after a partial capture. | +| `SecondClientOffered` | `second_client_offered` | `bool` | mark endpoint | Sticky. | +| `SecondClientNoneDetected` | `second_client_none_detected` | `bool` | mark endpoint | Sticky. | +| `SecondClientSkipped` | `second_client_skipped` | `bool` | mark endpoint | Sticky. | +| `SecondClientConnected` | `second_client_connected` | `bool` | `observeConnectedClients` | Sticky. True once `len(ClientFirstObservedAt) >= 2`. | +| `ClientFirstObservedAt` | `client_first_observed_at` | `map[string]time.Time` | `observeConnectedClients` | Keys are restricted to registry client IDs (`internal/connect/clients.go`). Entries are added and never overwritten. Local only. | + +### Derived values (not stored) + +| Value | Derivation | +|---|---| +| `clients_day0` | Count of `ClientFirstObservedAt` entries whose time is in `[InstallFirstRunAt, +24h)`, bucketed `0 / 1 / 2 / 3_plus`. `""` when `InstallFirstRunAt` is zero. | +| `clients_day0_final` | `now >= InstallFirstRunAt + 24h`, when there is an anchor. | +| `approval_abandoned` | Checked on the telemetry-independent facts tick (R5a): `StarterCapturedAt` set (successful capture), `now - StarterCapturedAt >= 24h`, `!StarterLive`, and the starter (same incarnation) is still quarantined. Then `StarterLastFailure = approval_abandoned`. A starter that never captured keeps its capture or connect failure. | + +## 2. `telemetry.ActivationState` additions (bbolt bucket `activation`) + +Base: `internal/telemetry/activation.go:17-50`. + +| Key / Field | Type | Rules | +|---|---|---| +| `first_tool_indexed_ever` | bool | Sticky. Written by `MarkFirstToolIndexed`. | +| `first_tool_indexed_at` | time, stored locally | Written once with the flag. **Never** on the wire. | +| `FirstToolIndexedAfter` (wire `first_tool_indexed_after`) | enum | Derived at payload build from `first_tool_indexed_at - InstallFirstRunAt`: `lt_10m`, `10m_1h`, `1h_24h`, `1d_7d`, `gt_7d`. `""` without an anchor or before the flag. | + +## 2a. In-memory evidence (not persisted) + +`factEvidence sync.Map[(kind ∈ {index, capture}, server, incarnation)] → first commit time`. Producers use `LoadOrStore`. The facts loop deletes an entry only after persisting it. Unmatched entries expire after 15 minutes (research R4). + +## 3. Activity log additions + +### 3a. `tool_quarantine_change` metadata (research R8) + +| Key | Type | Values | +|---|---|---| +| `entered_hold` | bool | true only on a not-held → pending/changed transition with quarantine in effect | +| `hold_kind` | string | `new`, `changed`, or absent | + +### 3b. `ActivityRecord.BlockReason` new values (research R7) + +The new values are `server_quarantined`, `tool_pending_approval`, `tool_changed_approval`, `tool_not_callable`, `profile_scope`, `token_scope`, `token_permission`, `output_sanitisation` and `output_schema`. The last two are post-dispatch. They are the existing telemetry block-reason keys. Until now they were dropped by `emitActivityPolicyDecision`; they are now persisted when the record has no profile reason. + +### 3c. Dispatch fact on `tool_call` completions + +Every `tool_call` record carries `Metadata["dispatched"]` (bool), taken from the existing dispatch evidence (research R6). This covers the completion funnel, limiter sheds (`RecordToolCallRejected`) and sandbox sub-call refusals. Brokered = `dispatched == true`. Records without the key are legacy and count heuristically, with partial coverage. + +### 3d. Coverage watermark + +| Key | Location | Rules | +|---|---|---| +| `activity_complete_since` | activity meta | Moves forward only. Each prune advances it in its own delete transaction. Initialised on first open (research R9). | + +## 4. Wire types + +### 4a. `contracts.StarterOffer` (`GET /api/v1/onboarding/starter`) + +All REST bodies use the standard `{"success": true, "data": …}` envelope (`internal/httpapi/server.go:1463`). The shapes below are the `data` member. + +```json +{ + "eligible": true, + "reason": "eligible", + "preselect": true, + "candidate": { + "kind": "time", + "catalog_id": "reference/time", + "command": "uvx mcp-server-time", + "isolation": "host", + "expected_tools": ["get_current_time", "convert_time"], + "first_prompt": "What time is it in Tokyo right now?" + }, + "launcher": "uvx", + "starter_server": null, + "facts": { + "offered": false, "added": false, "live": false, + "declined": false, "last_failure": "" + } +} +``` + +- `reason` is one of `eligible`, `launcher_missing`, `name_taken`, `reference_overridden`, `tools_present`, `held_tools`, `already_configured` or `server_edition`. +- `candidate` is null when no candidate passes. +- `isolation` is `host` or `docker`, taken from the effective isolation decision for that config. +- `starter_server` is `{name, quarantined}` once a starter is configured. Surfaces use it to resume the review step after a reload. + +### 4b. `contracts.ActivityDigest` (`digest` in `ActivitySummaryResponse`) + +```json +"digest": { + "calls_brokered": {"total": 1284, "by_client": [{"client": "Claude Code", "count": 812}, {"client": "Codex", "count": 431}], "other": 41, "unknown": 0, "coverage": "complete"}, + "held_for_review": {"entered": 3, "new": 2, "changed": 1, "currently_awaiting": 2, "coverage": "complete", "reason": null}, + "blocked": {"total": 5, "quarantine": 4, "profile": 1, "other": 0, "responses_blocked": 1}, + "sensitive_data": {"calls_with_detections": 2}, + "coverage": {"state": "complete", "complete_since": "2026-09-01T00:00:00Z"}, + "window": {"start": "2026-10-13T12:00:00Z", "effective_start": "2026-10-17T09:30:00Z", "label": "since_install", "days": 3} +} +``` + +- `window` is the single shared source for the US4 scenario 6 label (plan review r3 #16). `start` is the nominal period start, the same value as the existing `start_time`. `effective_start = max(start, install anchor)`. `label` is `since_install` when `effective_start > start`, otherwise `period`. `days` is the whole number of days from `effective_start` to the end, rounded up. An anchorless (upgraded) install always gets `label = period`. Coverage is independent: a `period` window can still be `partial`. +- `blocked.responses_blocked` counts post-dispatch output blocks paired by `request_id` with a dispatched `tool_call`. Blocked prompt results never count. They are not in `blocked.total`, because those calls were brokered. +- `coverage` comes from the installation-wide watermark for admin callers only. Scoped callers get `{state: "unavailable", complete_since: null}`, so out-of-scope pruning cannot leak through it. +- `held_for_review` takes the form `{entered: null, ..., reason: "not_attributable"}` when an attribution filter is set (spec FR-023). +- `held_for_review.coverage` is `partial` while legacy records without `entered_hold` exist in the window. +- `new + changed` may exceed `entered`: a tool counts once in `entered` but may appear under both kinds if it entered twice with different kinds. + +### 4c. `telemetry.DayTwoPayload` (`day_two` in the heartbeat, schema 17, personal edition only) + +```json +"day_two": { + "starter_eligibility": "eligible", + "starter_launcher": "uvx", + "starter_offered": true, "starter_added": true, "starter_live": true, "starter_declined": false, + "starter_last_failure": "", + "starter_kind": "time", + "starter_surface": "wizard", + "second_client_offered": true, "second_client_none_detected": false, + "second_client_skipped": false, "second_client_connected": true, + "clients_day0": "2", + "clients_day0_final": false, + "summary_views": {"web": "1", "tray": "0", "cli": "0"} +} +``` + +The enums are closed. The PII scan rejects unknown keys, unknown values, non-boolean flags, and any timestamp or name. + +## 5. State transitions + +### Starter (local) + +``` +not evaluated ──first fresh-install check──▶ eligibility recorded +offer rendered ──mark──▶ offered +"Add and inspect" ──POST /onboarding/starter──▶ added ──index hook──▶ live + │ + ├─▶ last_failure = capture_timeout / connect_failed (UI mark), retry allowed + └─▶ last_failure = approval_abandoned (24 h, core) +Skip / uncheck ──mark──▶ declined (pre-selection off; Home shows a compact link) +``` + +All booleans are sticky. `live` dominates for success analysis, and failures are diagnostic only. + +### Second client (local) + +These four booleans are independent: `offered`, `none_detected`, `skipped` and `connected`. Only `connected` is derived by the core; the others are UI marks. None of them is ever cleared. diff --git a/specs/123-day-two-activation/plan.md b/specs/123-day-two-activation/plan.md new file mode 100644 index 000000000..d26572a39 --- /dev/null +++ b/specs/123-day-two-activation/plan.md @@ -0,0 +1,274 @@ +# Implementation Plan: Day-Two Activation (Experiment E3) + +**Branch**: `123-day-two-activation` (design; PR branch `claude/spec-day-two-activation-1fad2a`) → implementation branches `123-pr-a-measure`, `123-pr-b-starter`, `123-pr-c-second-client`, `123-pr-d-digest`, `123-pr-e-native` | **Date**: 2026-10-20 | **Spec**: [spec.md](spec.md) +**Input**: spec.md, which the OpenCode Sol 6.1 review approved in round 3, and research.md R1-R12. + +## Summary + +Five PRs. PR-A (measurement) goes first. PR-B, PR-C and PR-D depend only on PR-A, and PR-E (native) depends on B, C and D. + +1. **PR-A: measure first (US5).** All local facts and telemetry, with no visible UI: + - Onboarding state additions: the install anchor, the client-observation map, and the starter and second-client facts. + - Index-commit stamping of `first_tool_indexed_ever`. + - Telemetry schema 17: `day_two`, the activation additions, and closed-object PII rules. + - Docs. + + Shipped alone, it yields a release of schema-17 data before exposure (R12). +2. **PR-B: starter (US1, US2 Web).** + - `internal/onboarding` starter allowlist and resolver (R1, R2). + - `GET/POST /api/v1/onboarding/starter` (R3). + - Wizard Servers-step starter card with inline bound approval. + - Web Home offer. + - Verify-step first prompt. +3. **PR-C: second client (US3 Web).** + - Clients-step "Connect another client" section, built on `ClientConnectList`. + - `detected_unconnected_clients` in the onboarding state. + - The mark-endpoint booleans. +4. **PR-D: weekly digest (US4 core + Web + CLI).** + - Hold-entry metadata (R8), typed block reasons (R7), proxy fault domain on refusals (R6) and the coverage watermark (R9). + - The `digest` aggregator and `view` counting (R10). + - Web "This week" card. + - CLI `--since` and digest table. +5. **PR-E: native macOS (US2, US3, US4 native).** + - Home empty-state starter card with handoff to the native review. + - The "Connect another client" success state. + - Glance-block weekly line. + - `ActivitySummary` model catch-up. + +## Technical Context + +**Language/Version**: Go 1.26 (core), TypeScript 5.9 / Vue 3.5 (Web UI), Swift 5.9 (macOS tray) +**Primary Dependencies**: existing only: bbolt, zap, chi, cobra, mark3labs/mcp-go, Vue/Pinia, AppKit/SwiftUI. **No new dependencies.** +**Storage**: BBolt `config.db`: +- additive fields on the `onboarding/wizard_state` record; +- two keys in the `activation` bucket; +- one activity meta key (`activity_complete_since`); +- two metadata keys on `tool_quarantine_change` records; +- new `BlockReason` values. + +No migration. Downgrade ignores the new fields. +**Testing**: +- `go test -race` in `internal/onboarding` (new), `internal/storage`, `internal/runtime`, `internal/telemetry`, `internal/httpapi`, `internal/shellwrap` and `cmd/mcpproxy`. +- The `internal/server` race lane under `-tags server`, with the CI skip regex (CLAUDE.md). +- vitest `frontend/tests/unit/*.spec.ts`. +- The Playwright sweep (`docs/development/web-ui-verification.md`). +- Swift `MCPProxyTests`, plus a `mcpproxy-ui-test` live check. +- `./scripts/test-api-e2e.sh`, only after checking `pgrep` for other sessions' cores. + +**Target Platform**: the personal edition (macOS, Windows, Linux) for all UI. The server edition gets only the additive summary fields, and `GET /onboarding/starter` there answers `server_edition`. +**Project Type**: web + desktop (Go core, Vue SPA, Swift tray). +**Performance Goals**: +- SC-006: 7-day digest under 1.5 s on 100k records, in a single stream pass. +- Starter offer endpoint under 50 ms warm (launcher cache 60 s, login-shell PATH cached). +**Constraints**: +- Additive wire changes only. +- No starter code runs before an explicit "Add and inspect". +- No approval without a bound user click. +- No bbolt write while holding the per-server approval lock (R4). +- The `day_two` object is enum-closed. +**Scale/Scope**: about 25 Go files, 8 Vue/TS files and 7 Swift files touched; 5 PRs. Estimated diff about 3.5k lines including tests. + +## Constitution Check + +*GATE: checked before Phase 0, re-checked after Phase 1.* + +| Principle | Status | Notes | +|---|---|---| +| I. Performance at scale | PASS | The digest adds one bounded buffer (error request IDs) to the existing single stream pass. No index-path cost beyond one non-blocking notify after a successful batch. SC-006 has a benchmark (T056). | +| II. Actor-based concurrency | PASS | One goroutine (`onboardingFactsLoop`) owns every fact write. It is woken by a size-1 channel and a ticker, and never writes under the approval lock. The summary-view debounce uses an atomic per surface. No new mutex. | +| III. Configuration-driven | PASS | No new config keys. Starter eligibility respects the effective `registries` config (`reference_overridden`) and `quarantine_enabled`. | +| IV. Security by default | PASS (with one inherited deviation, see Complexity Tracking) | Quarantine, trust mode and approval semantics are unchanged (FR-003, FR-005, SC-005). The starter is never auto-approved. Informed consent comes before code runs (FR-004). Docker isolation follows the global setting. | +| V. TDD | PASS | Every task group starts with failing tests (tasks.md). | +| VI. Documentation hygiene | PASS | `docs/features/telemetry.md`, `docs/cli/activity-commands.md`, `docs/web-ui/dashboard.md`, `docs/features/connect-clients.md`, `docs/features/activity-log.md`, `oas/swagger.yaml`, plus a CLAUDE.md "Recent Changes" line per PR. | +| Core + tray split | PASS | The tray reads the offer and digest over REST and holds no state. All eligibility logic is in the core (FR-012). | +| Event-driven updates | PASS | Onboarding fact changes emit the existing `onboarding` SSE refresh. No polling is added; the Home card refreshes on the existing activity SSE. | +| DDD layering | PASS | The new `internal/onboarding` holds the pure domain logic (allowlist, eligibility, bucket math). It sits behind a small interface implemented by `internal/server` (catalog door, server list) and `internal/storage` (state). | + +No violations, so Complexity Tracking is empty. + +## Project Structure + +### Documentation (this feature) + +```text +specs/123-day-two-activation/ +├── spec.md # approved spec (Sol r3) +├── plan.md # this file +├── research.md # R1-R12 decisions +├── data-model.md # persisted and wire shapes, transitions +├── quickstart.md # live checks L1-L5 + E3 telemetry queries +├── contracts/ +│ ├── onboarding-starter.openapi.yaml +│ ├── activity-digest.openapi.yaml +│ └── telemetry-day-two.schema.json +├── checklists/requirements.md +└── tasks.md # /speckit.tasks output +``` + +### Source code (repository root) + +```text +internal/onboarding/ # NEW pure domain package (PR-A facts math, PR-B starter) +├── starter.go # allowlist (R1), Candidate, Select() +├── starter_test.go # static audit test (FR-002), selection order, fail-closed cases +├── eligibility.go # Eligibility(inputs) → (reason, preselect); launcher enum +├── facts.go # sticky-bool merge, day-0 bucket, first-indexed-after bucket +└── facts_test.go +internal/shellwrap/shellwrap.go # + ResolveOnEffectivePATH (R2) +internal/storage/ +├── models.go # OnboardingState additions (data-model §1) +├── bbolt.go # install anchor on first Get; observeConnectedClients helper +├── activity.go # coverage watermark in the 3 prune txs (R9) +├── activity_digest.go # NEW DigestAccumulator, BlockClass, brokered predicate (R6-R8) +└── activity_digest_test.go # + testdata/digest_fixture.json, digest_expected.json +internal/telemetry/ +├── telemetry.go # SchemaVersion 15, DayTwoPayload, SetDayTwoProvider +├── activation.go # first_tool_indexed_ever / _at, MarkFirstToolIndexed +├── anonymity.go # scanDayTwo + activation enum rule +├── day_two.go # NEW summary-view counter + bucketing +└── payload_v17_test.go # NEW; schema pins moved 16→17 (R11) +internal/runtime/ +├── absent_tool_removal.go # post-success non-blocking wake (R4) +├── onboarding_facts.go # NEW onboardingFactsLoop: observe clients, reconcile live/indexed, abandonment +├── tool_quarantine.go # entered_hold / hold_kind on emits (R8) +└── activity_service.go # metadata passthrough +internal/server/ +├── server.go # providers wiring; EnsureInstallAnchor at startup; onboardingFactsLoop start +├── add_from_registry.go # split into resolveRegistryEntry + addResolvedRegistryEntry (public door unchanged) +├── mcp.go # typed BlockReason persisted; metadata.dispatched on every tool_call completion (R6, R7) +└── onboarding_starter.go # NEW: resolve once, validate snapshot, addResolvedRegistryEntry, record facts +internal/httpapi/ +├── onboarding.go # GET/POST /onboarding/starter; mark booleans; observe clients; day_two in state +├── activity.go # digest + view param + scope rules (FR-023) +└── connect.go # observeConnectedClients on connect/disconnect +internal/contracts/{onboarding.go,activity.go} # StarterOffer, ActivityDigest +internal/profile/testdata/contract/{labels,refusals}.json # new BlockReason labels (R7) +cmd/mcpproxy/activity_cmd.go # --since alias, view=cli, digest table +frontend/src/ +├── components/onboarding/StarterOfferCard.vue # NEW (wizard + Home variants) +├── components/onboarding/SecondClientSection.vue # NEW +├── components/WeeklyDigestCard.vue # NEW +├── components/OnboardingWizard.vue # Servers/Clients/Verify integration +├── views/Home.vue # starter offer + digest card +├── stores/onboarding.ts, services/api.ts, types/api.ts +frontend/tests/unit/{starter-offer-card,second-client-section,weekly-digest-card,onboarding-wizard-starter}.spec.ts +native/macos/MCPProxy/MCPProxy/ +├── API/APIClient.swift, API/Models.swift # starter offer, digest, ActivitySummary catch-up +├── Views/HomeView.swift + Views/StarterOfferCard.swift (NEW) +├── Views/ConnectClientView.swift # success-state "Connect another client" +└── Menu/Glance/GlanceSection.swift # weekly line +native/macos/MCPProxy/MCPProxyTests/{StarterOfferTests,WeeklyLineTests,ConnectAnotherClientTests}.swift +oas/swagger.yaml; docs/** (see Constitution VI) +``` + +**Structure decision**: the existing layout, plus one new pure package, `internal/onboarding`. It keeps the eligibility, allowlist and bucket rules out of `httpapi`, so the Web and native surfaces share them through REST, and the rules are unit-testable without a server. + +## Design Notes + +### D1. Starter flow sequence (wizard) + +``` +Web/native core +GET /onboarding/starter ───────▶ onboarding.Select(effective registries, PATH, servers, quarantine state) + + record starter_eligibility / starter_launcher (first fresh check) +render card (command, isolation) +POST /onboarding/mark {starter_offered} +click "Add and inspect" +POST /onboarding/starter {kind, surface, shown_command} ─▶ requireServerOp(AddFromRegistry) + entry := resolveRegistryEntry("reference", id) // once + validate entry (builtin/reference, audited cmd == shown_command) else 409 starter_changed + addResolvedRegistryEntry(ctx, entry, {}) // the door's add step, unchanged + record added/kind/surface/server_name/incarnation; wake facts loop +poll GET /servers/{name}/review (2 s, ≤90 s or 180 s docker; SSE servers.changed also triggers) +render captured tools; diff check vs expected_tools (FR-005 notice) +click "Approve and enable" → POST /servers/{name}/security/approve {expected_hashes, expected_incarnation} + (Spec 117 path; 409 review_out_of_date → reload) +index commit → wake → onboardingFactsLoop reconcile → starter_live, first_tool_indexed_ever +Verify step: tool count + first prompt +``` + +### D2. Fact evidence, the facts loop and Spec 117 locks (research R4, R5, R5a) + +**Producers** run at the commit sites and never touch storage: +- After `Runtime.batchIndexTools` (`absent_tool_removal.go:150-157`) succeeds, the discovery apply passes its own incarnation in, and the producer records `LoadOrStore((index, server, incarnation), now)`. Its callers are `lifecycle.go:1753,1946,1967,1982`, under the Spec 117 per-server approval lock. +- At capture completion (`lifecycle.go:~1066`, after both failure returns), it records `LoadOrStore((capture, server, incarnation), now)`. +- Either producer then makes a non-blocking send on a size-1 wake channel. + +**`onboardingFactsLoop`** is the only fact writer. It wakes on the channel, after the starter POST, on a 10-minute tick and at startup. On each pass it: +- copies the evidence map; +- reads the connect service outside any transaction; +- applies one pure `UpdateOnboardingState` merge. Inside it, the starter association `(name, incarnation)` is compared with the evidence **in the transaction**; +- writes activation; +- deletes the evidence entries only after a successful write. On failure it retries on the next tick. + +It runs whatever the telemetry state is and drains once on shutdown, bounded to 2 s. + +**Starter identity** comes from the create commit's `CommitResult` incarnation, threaded out of `addResolvedRegistryEntry` (`config_cow.go:70-76` discards it today). + +**Tests**: +- no storage write under the approval lock (`commit_testhooks.go`); +- index commit then purge before the loop runs: the fact still lands, at the commit time; +- evidence before registration; +- interleaved same-name replacement: no false live; +- partial capture failure: no capture evidence; +- write failure retries. + +### D3. Digest single pass (R6-R8) + +`DigestAccumulator.Add(rec)` is called inside the existing `for a := range StreamActivities(filter)` loop in `handleActivitySummary`. It keeps: +- per-client counts; +- a set of blocked policy-decision request IDs; +- the error tool_calls' request IDs, pending resolution; +- a `(server, tool)` set for hold entries; +- a legacy-hold flag; +- sensitive-data and blocked-class counts. + +Brokered is `tool_call && metadata.dispatched == true` (research R6). A pre-dispatch blocked `policy_decision` counts in `blocked`. A `tool_call` with `dispatched == false` and no decision for its `request_id` counts once, classified by `BlockClass` on its own record, for example a sandbox profile refusal or a limiter shed. This needs a buffered ID set resolved in `Finish`. Post-dispatch blocks (`output_sanitisation`, `output_schema`) count in `responses_blocked`. `Finish(scope)` It also adds `currently_awaiting` from `Runtime.GetReviewQueue`, filtered by `filter.AllowedServers`, and coverage from the watermark. `extractSensitiveDataInfo` is exported as `SensitiveDataDetected(rec)`. `Finish` also fills `window`: nominal start, `effective_start = max(start, install anchor)`, and the label kind and days. Every surface renders the window label from this one source. + +### D4. Scope rules (FR-023) + +Scoped callers already have `filter.AllowedServers` applied to the stream (`internal/httpapi/activity.go:161-170`), so every stream-derived figure is in scope. `currently_awaiting` is filtered by the same allowed set. When `filter.Profile`, `filter.ClientID` or `filter.TokenName` is set, `held_for_review` is returned with null figures and `reason: not_attributable`. Test: a scoped token on servers {a, b} with holds on c gives an identical response with and without c's holds. + +### D5. Web surfaces + +- `StarterOfferCard.vue` has two modes. `wizard` is pre-selected, with Skip; `home` has an explicit button and a compact-link variant after a decline. It owns the add → poll review → bound approve state machine (states `idle`, `adding`, `capturing`, `review`, `approving`, `live`, `failed`). +- Wizard tab completion is **unchanged**. The Servers tab completes only on `hasUsableServer` (`OnboardingWizard.vue:975-981`, backend `onboarding.go:328-336`), so a quarantined or uncaptured starter never completes it. Only an approved, indexed starter does. A vitest covers this. +- `SecondClientSection.vue` reuses `ClientConnectList` filtered to `detected_unconnected_clients` in a new **`per-client` mode**. This mode hides the bulk-connect footer and its loop (`ClientConnectList.vue:470-486,1275-1286`), so every write needs that client's own preview confirmation (FR-014). The preview → write → undo flow and the diff and backup notice come from the existing component. A vitest asserts that no bulk action renders in this mode. +- `SecondClientSection` and `WeeklyDigestCard` render only in the personal edition, guarded on the `/api/v1/status` `edition`. The digest endpoint itself is edition-neutral. +- `WeeklyDigestCard.vue` sits on Home between `AttentionList` and `UsageSummaryStrip`. It sends `view=web` once per mount when visible, then refreshes without `view` on activity SSE. + +### D6. Native surfaces + +- `StarterOfferCard.swift` mirrors the Web card's home mode. A new `StarterFlowModel` runs the same state machine as the Web card: capture wait, inline captured review, audited-inventory mismatch notice, Retry/Remove and quarantine-off ready. It never offers the blind approval that the ordinary native review allows before capture (`ReviewQueueView.swift:244-254`). The full native review opens only on request. Today that screen sends `expected_hashes` but **no** `expected_incarnation` (`API/APIClient.swift:290-298`). PR-E adds incarnation decoding to the native review model and passes it from `securityApproveServer` (research R3). A remove and re-add before approval must return 409 `server_replaced`. +- `ConnectClientView` success state: the list comes from `GET /onboarding/state`'s `detected_unconnected_clients`, mapped through the existing connect client list. The success state sends the `second_client_offered`, `second_client_none_detected` and `second_client_skipped` marks through a new `markOnboarding` method on the data-source seam. +- `GlanceSection` weekly line: `activitySummary(period: "7d", view: nil)` is fetched on the existing glance refresh. `menuWillOpen` sends one `view=tray` request, debounced by the core. +- The `ActivitySummary` Swift model adds `call_count`, `call_error_count`, `rejected_count`, `other_count` and an optional `digest`. Older cores omit them, so they decode as optional. + +### D7. PR boundaries and order + +| PR | Contains | Depends on | Ships | +|---|---|---|---| +| PR-A | facts, facts loop, telemetry, docs (tasks.md Phases 1-2) | none | first, alone if the calendar allows (R12) | +| PR-B | starter core + Web (tasks.md Phase 3) | PR-A | next release | +| PR-C | second client Web (tasks.md Phase 4) | PR-A | with PR-B | +| PR-D | digest core + Web + CLI (tasks.md Phase 5) | PR-A for view counting; it otherwise stands alone | with PR-B | +| PR-E | native (tasks.md Phase 6) | PR-B, PR-C, PR-D | same release if ready, otherwise the next | Merge each PR on its own, with no stacking and at most two reviews in flight (memory: merge early, don't stack). Each PR gets the OpenCode Sol 6.1 review loop, with the 10-round cap. + +### D8. Risks + +| Risk | Mitigation | +|---|---| +| New `BlockReason` values break the Spec 108 refusal and label parity contracts or Activity filters | Update the contract JSON and labels in PR-D. Run the profile contract tests and the Activity view vitest. | +| A completion path forgets to set `dispatched` | A table test drives every `tool_call` completion exit in `mcp.go` (pre-dispatch validation, missing client, generation refusal, success, upstream error, both output blocks, strict output schema) and asserts the key. Legacy-only windows report `coverage: partial`. | +| An index or capture commit happens and its documents are purged before the loop runs | Commit-time evidence is retained in memory until it is persisted (research R4). A purge-before-consume test covers this. | +| The registry changes between the consent check and the add | One resolved entry snapshot is validated and passed to the add step (research R3). A swap-between-steps test covers it. | +| The native approval binds to the wrong server incarnation | PR-E sends `expected_incarnation` (research R3). A remove-and-re-add test covers it. | +| The launcher is resolvable but the first `uvx` download is slow or offline | 90 s/180 s capture timeout, Retry/Remove, and `capture_timeout` recorded (SC-003 diagnostic). | +| A starter package release adds a tool | FR-005 "differs from reviewed version" notice. The user approves what was captured (Spec 117 binding). | +| Users never approve (the approval click becomes the new drop-off) | `approval_abandoned` is measured. If it is high, O1/O4 and copy changes are the next experiment. Auto-approval is never added. | + +## Complexity Tracking + +| Violation | Why needed | Simpler alternative rejected because | +|---|---|---| +| Constitution IV says "stdio MCP servers MUST run in Docker containers by default". The product default is `docker_isolation.enabled: false` (`internal/config/config.go:1644-1646`, "for backward compatibility"), so the starter runs as a host process by default. | **Pre-existing deviation, not introduced here.** The starter follows the global isolation setting, exactly like every catalog add (FR-003). The consent card shows the isolation mode before anything runs (FR-004). Both candidates are audited as pure-computation tools (FR-002). | Forcing Docker for the starter alone would need Docker on a fresh machine. That defeats "no Docker needed" and adds a starter-specific trust path that FR-003 forbids. Flipping the global default is a product decision outside E3; it is filed as follow-up O5 in the PR body. | diff --git a/specs/123-day-two-activation/quickstart.md b/specs/123-day-two-activation/quickstart.md new file mode 100644 index 000000000..aaa06a2df --- /dev/null +++ b/specs/123-day-two-activation/quickstart.md @@ -0,0 +1,130 @@ +# Quickstart: verifying Spec 123 (Day-Two Activation) + +Live checks run on an isolated scratch instance: high port, scratch data dir, own log dir. Never run them against the maintainer's live app. Kill any scratch core before you rebuild, because the core locks the DB. + +```bash +go build -o /tmp/s123/mcpproxy ./cmd/mcpproxy +mkdir -p /tmp/s123/data /tmp/s123/logs +echo '{"listen":"127.0.0.1:18919","api_key":"s123-test-key","mcpServers":[]}' > /tmp/s123/mcp_config.json +# --config is mandatory: --data-dir alone still reads ~/.mcpproxy/mcp_config.json +MCPPROXY_TELEMETRY=false /tmp/s123/mcpproxy serve --config /tmp/s123/mcp_config.json \ + --data-dir /tmp/s123/data --log-dir /tmp/s123/logs & +# Stop only this instance: pkill -f 'mcpproxy serve.*s123' +set -euo pipefail # every assertion below stops the run on failure +K='X-API-Key: s123-test-key'; B=http://127.0.0.1:18919/api/v1 +``` + +## L1. Starter offer → add → bound approval → indexed (US1, SC-005) + +```bash +curl -s -H "$K" $B/onboarding/starter | jq '.data | {eligible,reason,preselect,launcher,candidate}' +# expect eligible=true, candidate.kind="time" (uv installed), preselect=true + +CMD=$(curl -s -H "$K" $B/onboarding/starter | jq -er .data.candidate.command) +curl -s -H "$K" -X POST $B/onboarding/starter \ + -d "{\"kind\":\"time\",\"surface\":\"wizard\",\"shown_command\":\"$CMD\"}" | jq .data.server +# expect quarantined=true + +# Before approval: tool_count stays 0 and a call is refused. +curl -s -H "$K" $B/servers | jq '[.. | .tool_count? // empty] | add' # 0 (indexed tools) +sleep 20; R=$(curl -s -H "$K" $B/servers/time/review) +echo "$R" | jq -e '(.data.tools | length) > 0 and all(.data.tools[]; (.current_hash // "") != "") and (.data.server.incarnation != null)' +echo "$R" | jq '[.data.tools[].name]' # ["convert_time","get_current_time"] +# SC-005: a call before approval is refused and never reaches the upstream +N0=$(curl -s -H "$K" "$B/activity?type=policy_decision&server=time&tool=get_current_time&status=blocked&limit=1000" | jq '.data.activities | length') +# If the core is on mcp-go >= v1.0, also send `Mcp-Method: ` on each POST. +# Use the daemon's MCP endpoint, NOT `mcpproxy call` (it falls back to a standalone mode that bypasses the daemon). +M=http://127.0.0.1:18919/mcp; HJ='Content-Type: application/json'; HA='Accept: application/json, text/event-stream' +SID=$(curl -s -D - -o /dev/null -H "$HJ" -H "$HA" $M -d '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"s123-qa","version":"0"}}}' | awk -F': ' 'tolower($1)=="mcp-session-id"{print $2}' | tr -d '\r') +curl -s -H "$HJ" -H "$HA" -H "Mcp-Session-Id: $SID" $M -d '{"jsonrpc":"2.0","method":"notifications/initialized"}' >/dev/null +curl -s -H "$HJ" -H "$HA" -H "Mcp-Session-Id: $SID" $M -d '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"call_tool_read","arguments":{"name":"time:get_current_time","args_json":"{\"timezone\":\"UTC\"}"}}}' \ + | tee /tmp/s123/refused.txt | grep -q '"isError":true' # must be refused +grep -qi 'quarantin' /tmp/s123/refused.txt # names quarantine +# The quarantine gate writes a policy_decision (no tool_call completion). Zero upstream dispatch is proven by the SC-005 safety test's upstream spy, not here. +N1=$(curl -s -H "$K" "$B/activity?type=policy_decision&server=time&tool=get_current_time&status=blocked&limit=1000" | jq '.data.activities | length') +[ "$N1" -eq $((N0 + 1)) ] # exactly one NEW blocked decision, for this attempt + +# Bound approval (Spec 117), as the wizard sends it. +H=$(echo "$R" | jq -ec '[.data.tools[] | {(.name): .current_hash}] | add') # hashes and incarnation were asserted non-empty above +I=$(echo "$R" | jq -e .data.server.incarnation) +curl -s -H "$K" -X POST $B/servers/time/security/approve \ + -d "{\"expected_hashes\":$H,\"expected_incarnation\":$I}" | jq -e .success +curl -s -H "$K" $B/servers | jq '[.. | .tool_count? // empty] | add' # >= 2 +curl -s -H "$K" $B/onboarding/state | jq .data.day_two # starter_added/live true, starter_kind "time" +``` + +**Negative checks:** +- Call `POST /onboarding/starter` with an agent token: 403, and no server is added (check `GET /servers`). +- Remove and re-add `time` between the review and the approval: the approval returns 409 `server_replaced`. +- Send `"shown_command":"uvx something-else"`: the response is 409 `starter_changed`, and nothing is added. +- Add `{"registries":[{"id":"reference","protocol":"custom/docker",...}]}` to the config: the offer becomes `reason=reference_overridden`. +- Put a user server named `time` with a different command in the config: `time` is skipped (`name_taken`), and `sequentialthinking` is offered if `npx` resolves. + +## L2. Web wizard and Home (US1, US2, US3) + +PR-B runs steps 2-4, PR-C runs step 1, and the combined sweep after PR-B, PR-C and PR-D merge runs all of them. + +Run the Playwright sweep (`docs/development/web-ui-verification.md`) against the scratch instance with an empty config: +1. The wizard auto-opens. The Clients tab, after one fake client connect (use the `MCPPROXY_FIXTURE_PATH` client fixtures), shows "Connect another client". +2. The Servers tab shows the pre-selected starter card with its command and "Add and inspect". +3. After the inline approval, the Verify tab shows the tool count and the first prompt. +4. Dismiss a second fresh instance's wizard without a starter. Home's "Get started" card offers "Add and inspect time", not pre-selected. + +Take screenshots for the PR. Check for no console errors. + +## L3. Weekly digest (US4) + +Seed the activity log with the fixture from task T052 (`internal/storage/testdata/digest_fixture.json`). It produces 10 days of records: two clients, a refusal pair, quarantine and profile blocks, holds that enter and re-enter, one already-held change, sensitive detections, and a prune that advances the watermark. Then: + +```bash +curl -s -H "$K" "$B/activity/summary?period=7d&view=cli" | jq .data.digest +# Pin the CLI to the scratch instance; without these it may query the live daemon via the default socket. +MCPPROXY_TRAY_ENDPOINT=http://127.0.0.1:18919 MCPPROXY_API_KEY=s123-test-key \ + /tmp/s123/mcpproxy activity summary --since 7d -o json \ + --config /tmp/s123/mcp_config.json --data-dir /tmp/s123/data | jq .digest # identical +``` + +The figures must equal the fixture's expected values (`internal/storage/testdata/digest_expected.json`). + +## L4. Telemetry payload (US5, SC-007) + +```bash +go test ./internal/telemetry -run TestPayloadV17 -v # T012: asserts and logs a sample schema-17 payload +go test ./internal/telemetry -run 'TestScanForPII.*DayTwo' -v +``` + +## L5. Native (US2, US3, US4) + +Use the tray dev-swap recipe (`docs/development/macos-tray.md`, `mcpproxy-ui-test`): +- the Home empty state shows the starter card; +- the Connect Client success state lists "Connect another client"; +- the glance block shows "This week: N calls · H held · B blocked". + +## Telemetry queries for judging E3 (run by the maintainer on D1) + +They use the canonical CI filter and identity keys from the 2026-10-20 telemetry note. `release_v` is the first build carrying PR-B, the starter UI. + +**SC-001: tools indexed, heartbeat-sampled, comparable with the 71.5% baseline.** + +```sql +WITH c AS (SELECT cont_id, MIN(created_at) first_seen FROM hb_noci + WHERE edition='personal' GROUP BY cont_id + HAVING MIN(version_semver) >= :release_v AND MIN(created_at) <= date('now','-14 day')) +SELECT AVG(ever_tools) FROM ( + SELECT c.cont_id, MAX(json_extract(payload,'$.tool_count') > 0) ever_tools + FROM c JOIN hb_noci h USING (cont_id) GROUP BY c.cont_id); +``` + +**SC-001 companion: sticky flag, reported separately.** Use the same query with `MAX(json_extract(payload,'$.activation.first_tool_indexed_ever'))`. + +**SC-003: launcher availability and starter conversion.** + +```sql +SELECT json_extract(payload,'$.day_two.starter_launcher') l, COUNT(DISTINCT cont_id) +FROM hb_noci WHERE schema_version >= 17 AND l != '' GROUP BY l; +-- conversion: share of cont_ids with any starter_offered=1 that ever have starter_live=1 +``` + +**SC-004: day-0 clients, same instrument on both sides.** Take the union of `connected_client_ids` over heartbeats with `created_at <= first_seen + 26h`, and compute the share with at least 2 distinct IDs among those with at least 1. Run it for pre-release cohorts (schema 16 or older) (baseline) and for schema-17 cohorts. + +**SC-002: D7 return on the IP key.** Use the existing retention script (`telemetry_data_method_scripts.py.txt`), restricted to cohorts first seen on version >= `:release_v`. diff --git a/specs/123-day-two-activation/research.md b/specs/123-day-two-activation/research.md new file mode 100644 index 000000000..6171f2bf2 --- /dev/null +++ b/specs/123-day-two-activation/research.md @@ -0,0 +1,226 @@ +# Research: Day-Two Activation (Spec 123) + +Every decision below resolves a design unknown from spec.md. Code references are to `origin/main` at `9dfdb2f19`. + +## R1. Which starter, and how it is resolved + +**Decision**: a compiled-in allowlist `internal/onboarding/starter.go`: + +| Order | Kind | Catalog ID | Audited command | Expected tools | +|---|---|---|---|---| +| 1 | `time` | `reference/time` | `uvx mcp-server-time` | `get_current_time`, `convert_time` | +| 2 | `sequentialthinking` | `reference/sequentialthinking` | `npx -y @modelcontextprotocol/server-sequential-thinking` | `sequentialthinking` | + +Each entry also records `audited_version`, the package version the maintainer reviewed, and a fixed `first_prompt`. + +A candidate is selected only if all of these hold: +1. The effective `reference` registry has protocol `builtin/reference`. A config entry with the same ID overrides it (`internal/registries/registry_data.go:9-14`). +2. `registries` resolves the catalog ID to exactly the audited command and args. +3. Its launcher resolves (R2). +4. No configured server uses its name. +5. No configured server already runs the same command. + +**Rationale**: the reference catalog is compiled in (`internal/registries/reference.go:26-74`) and has no required inputs. Docker isolation is off by default (`internal/config/config.go:1644-1646`). Both candidates are pure computation: no file, environment or network access at call time. + +**Alternatives rejected**: +- `everything`: its live inventory includes `get-env`, which returns all environment variables, plus LLM-sampling test tools. +- `fetch`: open-world URL fetching. +- `filesystem` and `git`: they read files. +- `memory`: it writes a file. +- An embedded in-process starter: deferred to O1, because it is a new server kind with its own lifecycle verbs (lesson MCP-2396). +- A version-pinned command: deferred to O4, because it would change the shared catalog entry. + +## R2. Launcher resolution without spawning + +**Decision**: a new `shellwrap.ResolveOnEffectivePATH(logger, name string) (string, bool)`. It runs an `exec.LookPath`-style search over `shellwrap.LoginShellPATH(logger)` (`internal/shellwrap/shellwrap.go:455`, cached) followed by `os.Getenv("PATH")`. On Windows it searches the process PATH only, matching what `LoginShellPATH` returns there. The result is cached for 60 s in the starter service. + +**Rationale**: stdio spawning wraps non-Docker commands in the login shell (`internal/upstream/core/connection_stdio.go:456` → `WrapWithUserShell`), so the login-shell PATH is the ground truth. `hasCommand` (`connection_stdio.go:474`) uses only the process PATH, which is degraded for GUI launches on macOS. + +**Alternatives rejected**: +- Spawning `uvx --version`: slow, has side effects, and counts as running code before consent. +- Process PATH only: wrong for Finder-launched cores. + +## R3. Starter add door, authorization and consent binding + +**Trust mode (2026-10-25)**: since Spec 119 (`specs/119-scan-default-trust`) the door resolves new servers to `scan` by default. The starter changes nothing about that, and the UI handles both outcomes: an unquarantined server after a clean admission scan, or a held server needing the bound in-place approval. + +**Decision**: +- `GET /api/v1/onboarding/starter` is wrapped in `requireAdminRead`. +- `POST /api/v1/onboarding/starter` is wrapped in the **same** authorization as the catalog door: `requireServerOp(auth.ServerOpAddFromRegistry, …)` (`internal/httpapi/server.go:1258`; agent tokens are denied by `internal/auth/server_ops.go:59-79`). +- Routed agent-token tests assert 403 with **no** resolution, add or fact write (plan review r1 #1). + +**Atomic consent binding** (plan review r1 #2). `AddServerFromRegistry` (`internal/server/add_from_registry.go:88`) is split into two steps: +1. `resolveRegistryEntry(registryID, serverID) (*registries.ServerEntry, provenance, error)`, the current `:93-99` logic; +2. `addResolvedRegistryEntry(ctx, entry, req)`, which runs the current `:100-141` build-and-add steps unchanged. + +The public catalog door calls both in sequence, so its behaviour is unchanged. The starter handler calls `resolveRegistryEntry` **once**, validates that snapshot (`builtin/reference` provenance, audited command equal to `shown_command`), then passes the **same** entry to `addResolvedRegistryEntry`. No second resolution happens, so a registry hot-reload between the check and the add cannot change what runs. A test swaps the `reference` registry between the starter check and the add, through a test hook between the two calls, and asserts that the validated command is what gets configured. + +- A mismatch with `shown_command` returns 409 `starter_changed` with a fresh offer, and nothing is added. +- `duplicate_name` from the add passes through as 400. + +**Approval** uses the existing `POST /api/v1/servers/{id}/security/approve` with `expected_hashes` **and** `expected_incarnation` (`internal/httpapi/security_scanner.go:~377`), both taken from the displayed `GET /api/v1/servers/{id}/review` (`internal/httpapi/review.go:68`, `data.server.incarnation`). +- **Web** (`frontend/src/services/api.ts:1641`) already sends both. +- **Native does not**: `securityApproveServer` sends only the hashes (`native/macos/MCPProxy/MCPProxy/API/APIClient.swift:290-298`), and the core then binds to whatever incarnation is current at call time (`internal/httpapi/security_scanner.go:350-360`). PR-E therefore adds `expectedIncarnation` to `securityApproveServer` and to the native review model. It is captured with the displayed review and tested with a remove and re-add before approval, which must return 409 `server_replaced` (plan review r1 #3). + +**Alternatives rejected**: clients calling `POST /registries/reference/servers/reference%2Ftime/add` directly. That loses the atomic consent check and the core-side fact recording. + +## R4. "Starter live" and "first tool indexed": commit-time evidence, retained until persisted + +**Decision**: "ever" facts come from **evidence captured at the commit that produced them**. The evidence is bound to the server incarnation and kept until it is persisted. A current-index read never derives them, apart from the one supplement noted below (plan review r1 #4/#5, r2 #15/#16). + +- **Evidence producers** run at the commit site, which already knows the server and its incarnation: + - **Index evidence**: after `Runtime.batchIndexTools` (`absent_tool_removal.go:150-157`) returns nil for a non-empty batch. The discovery apply that calls it (`lifecycle.go:1753,1946,1967,1982`) runs under the Spec 117 per-server approval lock, with Spec 116's incarnation-conditional writes. The apply passes its own incarnation into the hook. + - **Capture evidence**: at the point where a quarantined review capture completes, right after `storeLastGoodLocked` on the review-capture path (`lifecycle.go:~1066-1068`). This is past both failure returns (`checkToolApprovalsHeld` and `pruneAbsentUndecidedApprovals`, `:1054-1063`). A partial capture that fails is never evidence (r2 #17). +- **Retention**: a `sync.Map` keyed by `(kind, server, incarnation)` keeps the **first** commit time per key. It uses `LoadOrStore`, so a later commit never moves the time. Storing is lock-free for producers, nothing is ever dropped, and there is no bbolt write under the approval lock. A non-blocking send on a size-1 wake channel then nudges the facts loop. +- **Consumer**: `onboardingFactsLoop` (R5a) drains a **copy** of the map and applies one `UpdateOnboardingState` callback plus one activation write. It deletes an entry only after the write succeeds. On a write failure the entry stays and is retried on the next tick. Inside the callback, which is the transaction-current state: + - `first_tool_indexed_ever` and `first_tool_indexed_at` come from the **earliest** index evidence of any upstream server, at its commit time. + - `starter_live` is set only if index evidence exists for exactly `(StarterServerName, StarterServerIncarnation)` as stored **in the same transaction**. + - `starter_captured_at` is set from capture evidence for that same pair. + - Evidence that matches no recorded starter is kept for 15 minutes, which covers evidence arriving before the starter POST records its registration (r1 #5). It is then discarded once `first_tool_indexed` has been persisted. +- **Supplement**: on an upgraded install with tools already indexed before this build, the loop sets `first_tool_indexed_ever = true` from the current index document count, read directly, with `first_tool_indexed_at` unknown. That gives `first_tool_indexed_after = ""`. `GetToolCount()` is never used, because of its cache and upstream fallbacks (`runtime.go:4061-4078`). +- **Creation identity** (r2 #16): `addResolvedRegistryEntry` returns the `CommitResult` incarnation of the create commit. Today `createServer` discards it (`internal/server/config_cow.go:70-76`), so it is threaded through an internal result type. The starter POST records that incarnation, so a post-add lookup can never pick up a replacement. +- **Shutdown**: one bounded final drain, at most 2 s. Evidence still in memory at a crash is lost. It is regenerated only if the server indexes again, which is accepted and documented. + +**Tests (T007, T026)**: +- successful indexing, then a purge before the loop runs: the fact still lands, at the commit time; +- evidence before registration; +- interleaved remove → same-name re-add → index of B while A is the recorded starter: A is not live; +- a partial capture failure produces no capture evidence; +- a write failure retries and keeps the entry. + +**Alternatives rejected**: +- A buffered event channel with drop-on-full: it loses facts, because discovery skips unchanged tools (`lifecycle.go:1807-1819`). +- Current-index reconciliation as the source: a purge loses history, and documents carry no incarnation (`internal/index/bleve.go:55-73`). +- Stamping in `index.Manager`: the index package has no storage dependency. + +### R4a. Approval abandonment (plan review r1 #11, r2 #17) + +`approval_abandoned` is set only when **all** of these hold: +- capture evidence for the recorded `(StarterServerName, StarterServerIncarnation)` has been persisted as `starter_captured_at`; +- `now - starter_captured_at >= 24h`; +- `!starter_live`; +- the starter, at the same incarnation, is still quarantined. + +A starter that never completed a capture keeps its `connect_failed` / `capture_timeout` and is never reclassified. + +## R5. Install anchor and client observation + +**Decision (anchor, plan review r1 #6)**: `Storage.EnsureInstallAnchor(now)` runs **once at startup**, in a single `Update` transaction, **before** the HTTP server, the connect service or the MCP initialisation can write onboarding state. +- No record: it creates one with `InstallFirstRunAt = now`. +- A record exists without the field: the install is an upgrade, and the anchor stays zero forever. + +As a safeguard, the "missing record" branch of `UpdateOnboardingState` (`internal/storage/bbolt.go:1283-1305`) also sets the anchor when it has to create the record. That covers tests and any writer that runs before startup init. The anchor is never back-filled on an existing anchorless record. Tests cover update-before-get, concurrent first writers, and upgrade. + +**Decision (observation)**: `ClientFirstObservedAt map[string]time.Time` is updated by `observeConnectedClients(ids)`. The IDs are collected **outside** the transaction from `connect.Service.GetConnectedIDs` (`internal/connect/connect.go:371-393`), then merged inside the pure update callback, which never does I/O (`bbolt.go:1272-1276`). Callers: +- the connect and disconnect handlers (`internal/httpapi/connect.go:332-360`); +- `computeOnboardingState` (`internal/httpapi/onboarding.go:276`); +- the facts lifecycle tick (R5a), which does **not** depend on telemetry. + +Only unseen IDs are added; existing entries are never overwritten. `second_client_connected` becomes true once there are at least two entries. + +**Rationale**: `anonymous_id` rotates yearly (`internal/telemetry/telemetry.go:1864-1877`). Connect timestamps overwrite (`connect.go:332-360`). `connected_client_ids` is a live read. + +**Caveat**: two clients observed at different times count as "connected" even if they were never configured at the same time. This is accepted, because the metric is "used a second client with MCPProxy". + +### R5a. Telemetry-independent facts tick (plan review r1 #7) + +A runtime background goroutine, `onboardingFactsLoop`, runs at startup (after `EnsureInstallAnchor`) and then every 10 minutes. Each tick runs client observation, `reconcileActivationFacts`, and the abandonment check (R4a). It runs whatever the telemetry state is: opt-out, non-semver dev builds, or before the first heartbeat (`internal/telemetry/telemetry.go:974-1025`). The heartbeat only **reads** the persisted facts. + +## R6. Digest: brokered-call predicate (plan review r1 #8, #9) + +**Decision**: persist an explicit **dispatch fact** and do not infer it from the fault domain. +- On every `tool_call` completion record, the server sets `Metadata["dispatched"] = true|false`. The value comes from the existing per-request dispatch evidence: `noteUpstreamDispatched` (`internal/server/mcp.go:3346`), and `callerr.Facts.Dispatched` from the transport recorder (`internal/callerr/classify.go:73-80`). +- Pre-dispatch exits record `dispatched=false`. Examples are argument validation (`mcp.go:3221-3226`), missing or disconnected clients (`:3246-3250,3319-3323`), and the review-not-recorded or generation refusals (`:3446-3475`). +- **Limiter sheds** (r2 #18) bypass the normal completion emitter (`mcp.go:3412-3417`). Their record is built in `ActivityService.RecordToolCallRejected` (`internal/runtime/activity_service.go:555-576`), which also stamps `dispatched=false`. Direct and sandbox sheds are both tested: each counts once in blocked (`other`), zero in brokered, and causes no legacy-coverage downgrade. +- **Sandbox sub-call refusals** (`internal/server/audit_funnel.go:517-526`, `mcp_code_execution.go:1289-1299`) record `status=blocked` with a typed profile `BlockReason` and `dispatched=false`. +- **Brokered call**: `Type == tool_call && Metadata.dispatched == true`, with any status. +- **Legacy records** without the key are counted with the old heuristic (`status == success`) and set `calls_brokered.coverage = partial` until the window holds only new-format records. +- **Sensitive data** counts brokered calls only. + +**Post-dispatch blocks** (r1 #9). `emitActivityPolicyDecisionWithBlockReason` already distinguishes them (`isPostDispatchBlockKey(reasonKey)`, `internal/server/mcp.go:991-998`: `output_sanitisation`, `output_schema`). R7 persists `reasonKey` as the typed `BlockReason`, so the phase is derivable: +- a **pre-dispatch** blocked `policy_decision` counts in `blocked.total` and its class; +- a **post-dispatch** block counts in a new `blocked.responses_blocked` and **not** in `blocked.total`, but **only** when its `request_id` pairs with a `tool_call` that has `dispatched=true`. The call itself stays brokered. `prompts/get` emits the same `output_sanitisation` reason after an upstream prompt fetch (`internal/server/output_sanitisation.go:286-289`, `mcp.go:1124-1132`) with no `tool_call`, so it never pairs and never counts (plan review r4 #9). A blocked-prompt fixture asserts zero contribution to every digest figure. + +The strict output-schema exit (`mcp.go:3610-3612,3653`) must also emit the canonical `tool_call` completion with `dispatched=true`; a test covers both output-block paths. + +Blocked-total dedupe: a pre-dispatch refusal that left both a `tool_call` (`dispatched=false`) and a `policy_decision` counts once, via the `policy_decision`. A `tool_call` with `dispatched=false` and **no** paired decision (same `request_id`) counts once and is classified by `BlockClass(rec)` on its own record: a sandbox profile refusal is `profile`, and a limiter shed is `other`. It defaults to `other` only when the record has no classified reason (r2 #19). + +**Alternatives rejected**: `FaultDomain`. Fault ownership is not dispatch: argument rejection is `client` and a missing client is `upstream` (`internal/callerr/classify.go:97-106`). + +## R7. Digest: blocked reason classes + +**Decision**: persist a typed reason on every refusal record. +- `emitActivityPolicyDecisionWithBlockReason` (`internal/server/mcp.go:989-1009`) writes `reasonKey` into `ActivityRecord.BlockReason` whenever its `blockReason` argument is empty. Today the key is dropped. +- **Sandbox quarantine and approval-lock refusals** (plan review r3 #11) compute `policyRefusalReasonKey(gate)` for the audit line but pass an empty reason to `emitSubCallRefused` (`internal/server/mcp_code_execution.go:958-965`). They now pass that key, so a sandbox quarantine refusal classifies as `quarantine` exactly as its direct-call twin does. Fixtures pair direct and sandbox refusals for server quarantine, pending approval and changed approval, and assert the same class. The keys come from `internal/telemetry/preflight_counters.go:45-56` plus the post-dispatch keys `output_sanitisation` and `output_schema`. + +Mapping table (`internal/storage/activity_digest.go`, `BlockClass(rec)`): + +| `BlockReason` / record | Class | +|---|---| +| `server_quarantined`, `tool_pending_approval`, `tool_changed_approval` | `quarantine` | +| `profile_*` (`internal/profile/contract.go:56-64`), `profile_scope` | `profile` | +| `token_scope`, `token_permission`, `tool_not_callable`, any other value, empty | `other` | +| `Status == rejected` (limiter) | `other` | +| `output_sanitisation`, `output_schema` (post-dispatch) | `responses_blocked`, which is not in the total | + +**Rationale**: FR-020 forbids free-text matching (`Metadata["reason"]`). + +**Risk**: the new `BlockReason` values become visible in the Activity UI and the Spec 108 contracts (`internal/profile/testdata/contract/{labels,refusals}.json`). PR-D adds labels and updates the parity tests in the same PR. + +## R8. Digest: held-for-review entries (plan review r1 #10) + +**Decision**: instrument the actual **not-held → held** transitions. +- `emitToolQuarantineEvent` (`internal/runtime/tool_quarantine.go:3363`) gains `enteredHold bool, holdKind string`, which become activity metadata. Each call site passes values computed from the record's **previous** and **new** status, plus whether quarantine is in effect for the server. + +| Site | Values | +|---|---| +| New record saved as pending, quarantine in effect (`:917-954`) | `true`, `new` | +| A status transition approved → changed (the description-change sites), quarantine in effect | `true`, `changed` | +| The same transition while quarantine is off (`:1582-1594`) | `false` | +| `refreshHeldContractSnapshot` (`:380-386,451-458`), already changed | `false` | +| `adoptLegacyLockOrBaseline`, where it **creates** a pending hold in the failed-scan and manual branches (`:2150-2167`). Today these emit nothing; a new emit is added | `true`, `new` | +| Legacy lock carry-over (`:2177-2194,2211-2225`, including the `:2262` emit), which preserves an existing pending or changed lock | `false` | +| Toggle path that synthesises a pending record (`:2721-2765`) | adds `tool_discovered` with `true`, `new` when it creates the pending record | +| `tool_auto_approved`, `tool_approved`, `tool_blocked` | `false` | + +- A table-driven test enumerates every `emitToolQuarantineEvent` call site by grep count, so a future site cannot be added without a decision. +- Fixtures cover both adoption helpers, carry-over, refresh of an already-changed tool, quarantine off, and enter → approve → re-enter. +- Digest coverage for this figure is `partial` while the window still contains `tool_quarantine_change` records without the field. + +**Alternatives rejected**: deriving from `ToolApprovalRecord.PendingSince` / `DefinitionChangedAt`. These are current-state fields (`internal/storage/bbolt.go:479-501`). + +## R9. Coverage watermark + +**Scope** (plan review r4 #8): the watermark is installation-wide, so pruning of a disallowed server's history would leak through it. Only **admin** callers get `coverage.state` and `complete_since` from it. A scoped caller (`AllowedServers` set) gets `coverage: {state: "unavailable", complete_since: null}`, which never depends on records outside its scope. Test: identical A/B activity with different C history (including a prune of C and watermark initialisation from a C record) gives byte-identical digests for an A/B-scoped token. + +**Decision**: a new key `activity_complete_since` in the existing activity meta area (`internal/storage/activity.go`). `PruneOldActivities` (`:342`), `PruneExcessActivities` (`:393`) and `PruneActivitiesToSize` (`:451`) each set it to the timestamp of the newest deleted record + 1 ns, inside their own delete transaction, and only ever move it forward. On first open by this build, if the key is absent, it is initialised to the oldest surviving record's timestamp, or to "now" for an empty log. + +**Rationale**: FR-021. Pruning deletes oldest-first in every mechanism, so "newest deleted" is a correct completeness boundary. + +## R10. Summary views and the `view` parameter + +**Decision**: +- `GET /api/v1/activity/summary` accepts an optional `view=web|tray|cli`. Any other value is ignored, with no error, to stay compatible. +- The handler calls `telemetry.RecordSummaryView(surface)`. This is an in-memory counter with a per-surface last-counted time (1 h debounce), drained into `day_two.summary_views` on an accepted send, like other delta counters (`internal/telemetry/registry.go:449-621`). +- Web sends `view=web` only when the Home card mounts with `document.visibilityState === 'visible'`, not on its 5-minute refresh. The tray sends `view=tray` only from `menuWillOpen`. The CLI always sends `view=cli`. + +**Rationale**: one aggregation path (A6). No client-side telemetry. + +## R11. Telemetry schema 17 + +**Decision**: +- `SchemaVersion = 17` (`internal/telemetry/telemetry.go:194` on 2026-10-25), with a change-log paragraph. +- `ActivationState` gains `FirstToolIndexedEver` (`activationKeyFirstToolIndexedEver`) and a derived `FirstToolIndexedAfter` bucket. The latter is computed at payload build from the stored `first_tool_indexed_at` and the install anchor, and is never stored as a timestamp on the wire. +- A new `DayTwoSnapshot` provider, mirroring `SetOnboardingProvider` (`telemetry.go:863`), builds `HeartbeatPayload.DayTwo *DayTwoPayload` (personal edition only; nil on server). The `day_two` keys are omitted when nil. +- `ScanForPII` gains `scanDayTwo`, modelled on `scanTrustModeDistribution` (`internal/telemetry/anonymity.go:317-357`). It enforces a closed key set, closed enums, booleans only for flags, and a closed `summary_views` object. It also covers `activation.first_tool_indexed_after`. +- Every pin of the then-current schema moves to the new version. On `origin/main` at 2026-10-25 the current schema is **16** (101-tpa-db added 15 and 16), so this feature bumps to **17**. Regenerate the pin list at implementation time with `grep -rnE 'schema_version.{0,3}16|want 16' internal/telemetry docs` and include `docs/features/telemetry.md`. If main has bumped again, use the next free version. + +**Rationale**: this follows the house convention for every schema bump. The receiver stores payloads wholesale, so unknown fields are accepted (`docs/features/telemetry.md:~348`). + +## R12. Release order: measure before exposing + +**Decision**: PR-A, which carries telemetry and the onboarding facts, merges first and ships in a release before the starter UI. This gives one release of schema-17 baseline data for `clients_day0` and `first_tool_indexed_ever` (spec SC-004). + +**Rationale**: SC-004's richer diagnostic has no pre-release baseline otherwise. The primary metrics (SC-001, SC-002) use pre-release-comparable instruments and do not depend on this. + +**Risk accepted**: if the starter and PR-A must ship in the same release because of the strategy calendar ("E3 shipped in the next release", W5-W8), the diagnostic baseline comes from the first release cohort. The plan allows that order. diff --git a/specs/123-day-two-activation/spec.md b/specs/123-day-two-activation/spec.md new file mode 100644 index 000000000..89da39089 --- /dev/null +++ b/specs/123-day-two-activation/spec.md @@ -0,0 +1,393 @@ +# Feature Specification: Day-Two Activation (Experiment E3) + +**Feature Branch**: `123-day-two-activation` (PR branch `claude/spec-day-two-activation-1fad2a`) +**Created**: 2026-10-20 +**Status**: Draft (design only; implementation not started) +**Input**: Growth experiment E3 from the Q4 2026 promotion strategy (maintainer-approved). Three product changes: (1) first run always ends with a working tool indexed, (2) the connect wizard offers a second client, (3) a weekly "what MCPProxy did" summary gives users a reason to come back. A fourth part adds the telemetry needed to judge the experiment. + +**Related**: Spec 075 (connect payload, `access_state`), Spec 108 (profiles, blocked-by-profile decisions), Spec 109 (Home needs-attention list, catalog-first Add, Clients hub), Spec 116 (`Runtime.Commit`), Spec 117 (review-bound approvals, `expected_hashes`), Spec 032 (tool-level quarantine), MCP-1072 (catalog provenance is informational only). + +All code references are to `origin/main` at `9dfdb2f19`. + +## Context and Motivation + +First-party telemetry (non-CI installs, 2026-03-23 to 2026-10-20) shows that retention, not setup, is the binding constraint: + +| Fact | Value | Source | +|---|---|---| +| New installs seen on one day only | 42% (machine key), 51% (IP key) | `telemetry_data_retention.csv` | +| Day-7 return | 23.9% (machine key), 14.9% (IP key), flat across Jun-Sep cohorts | same | +| Aug-Oct cohort: configured a server / connected a recognised client | 94.9% / 78.9% | `telemetry_data_funnel.csv` | +| Aug-Oct cohort: ever had a tool indexed (`tool_count > 0`) | 71.5% | same | +| One-and-done rate, tools indexed vs never indexed | 35.6% vs 65.9% | same | +| Machines running 2+ recognised clients | about half; Claude Code 44%, Codex 30%, OpenCode 26%, Cursor 20% of 30-day actives | `telemetry_data_clients_30d.csv` | + +What the product does today: + +| Area | Today on main | Code | +|---|---|---| +| Web onboarding wizard | Modal with Clients, Servers and Verify tabs. The Clients tab is complete once **any** client is connected. The wizard never auto-opens again after the first dismissal. | `frontend/src/components/OnboardingWizard.vue` (tabs ~:967, `pickInitialTab` ~:1146), `internal/httpapi/onboarding.go:276,342` | +| Starter server | None. A fresh install with zero servers sees a "Get started" card on Home. | `frontend/src/views/Home.vue:10-40` | +| Catalog | Curated reference servers are compiled in: filesystem, memory, everything, sequentialthinking, fetch, git, time. They need no credentials. Runtime is implicit in the install command (`uvx` or `npx -y`). | `internal/registries/reference.go:26` | +| Quarantine on add | Every catalog add is quarantined unless quarantine is globally off. Provenance never bypasses quarantine. **Update 2026-10-25**: Spec 119 (`specs/119-scan-default-trust`, implemented on main) makes `scan` the default trust mode. A clean admission scan now unquarantines a new server through the normal door, and `manual` always holds. A new quarantined server gets a 1-hour inspection exemption, so it connects and its tool definitions are captured for review. | `internal/config/config.go:2060,2173`, `internal/server/add_from_registry.go:~120-131`, `internal/server/server.go:2059`, `internal/server/review_capture.go:28` | +| Docker isolation | Off by default. | `internal/config/config.go:1644-1646` | +| "Tools indexed" | `tool_count` = search-index document count, sampled when the heartbeat is built. Quarantined or held tools are not in the index. | `internal/runtime/runtime.go:~4026`, `internal/telemetry/telemetry.go:189` | +| Activity summary | `GET /api/v1/activity/summary?period=1h|24h|7d|30d` and `mcpproxy activity summary --period` exist. They return totals, status counts, top servers and tools, and per-server counts. They have no held-for-review, blocked-by-reason, by-client or sensitive-data breakdown. | `internal/httpapi/activity.go:~886`, `internal/contracts/activity.go:132`, `cmd/mcpproxy/activity_cmd.go` | +| Tray | The AppKit menu has a glance block with a 24h activity line and an "Open Activity…" row. There is no weekly view. The native app has no onboarding wizard and never calls `/onboarding/*`. | `native/macos/MCPProxy/MCPProxy/MCPProxyApp.swift:~1041-1450`, `Menu/Glance/GlanceSection.swift` | +| Telemetry | Schema 16 on `origin/main` as of 2026-10-25; it was 14 when this spec was drafted, and 101-tpa-db added 15 and 16. This feature therefore bumps to schema 17. It reports wizard fields (`wizard_engaged`, `wizard_connect_step`, `wizard_server_step`, `wizard_shown`), `connected_client_count` / `connected_client_ids` and `activation.*` first-ever flags. It has nothing for a starter offer, a second-client step, summary views, or a sticky "a tool was ever indexed" flag. | `internal/telemetry/telemetry.go:163-190`, `internal/telemetry/activation.go:42` | + +The experiment's success bar comes from the strategy report: +- at least 82% of post-release new installs reach "tools indexed"; +- day-7 return on the IP key reaches at least 18%. + +The kill rule: no movement beyond noise after two releases and four weeks of cohort data. + +## Clarifications + +### Session 2026-10-20 + +Answered by the spec author under the zero-interruption policy; each answer is the recommended option, recorded as an assumption. + +- Q: Does "held for review" count events or tools? → A: Distinct `(server, tool)` pairs that actually entered `pending` or `changed` (from a non-held state, with quarantine in effect) during the window; a tool that flips twice counts once. (Revised after spec review r2 #3.) +- Q: Which records count as "calls brokered"? → A: Upstream `tool_call` records that were actually dispatched to an upstream (success, or an error after dispatch). A call refused before dispatch counts under "blocked" only, never twice; `rejected` (limiter) calls count under blocked/`other`. (Revised after spec review r1 #8.) +- Q: What if a server named like the starter (`time`) already exists with a different command? → A: That candidate is skipped with reason `name_taken` and the next candidate is tried; the starter never renames or overwrites a user server. +- Q: When the wizard opens with a client already connected (connected before or outside the wizard), is the second-client section shown? → A: Yes, immediately, listing the remaining detected clients. +- Q: What counts as a summary "view" for telemetry? → A: A summary request that names its surface (`view=web|tray|cli`): Web when the Home card renders in a visible tab, tray when the menu opens with the line shown, CLI per `activity summary` run. The core debounces to at most one view per surface per hour. + +## Scope Boundary + +| In scope | Out of scope (unchanged, or a follow-up) | +|---|---| +| A **starter offer** in the Web wizard's Servers step: one safe catalog server, pre-selected, added through the normal catalog door with normal quarantine. When a review is needed, the user approves it in place. | Any new trust shortcut. A starter is never auto-approved and never quarantine-exempt by virtue of being a starter. | +| The same starter offer, not pre-selected, on the Web Home "Get started" card and the native macOS Home empty state, whenever the install has zero indexed tools. | A starter on the CLI. `mcpproxy catalog add reference/time` already exists. | +| A **"Connect another client"** step after the first client connects, in the Web wizard and the native Connect Client sheet. It lists detected, supported, unconnected clients from the Spec 075 registry. | Auto-writing any client config. Every write stays an explicit, previewed action. | +| A **weekly "what MCPProxy did" summary**: a Web Home card, a tray glance line, and the CLI `mcpproxy activity summary --since 7d`. It is computed locally from the activity log and the current review queue. | Push notifications or e-mail digests (follow-up O2). | +| Additive **telemetry**: schema 17, with bucketed counters and closed enums that show starter offer and outcome, second-client outcome, clients connected on day 0, a sticky first-tool-indexed flag, and summary views. | New identifiers, server/tool/client names beyond the existing closed `connected_client_ids` list, and receiver or dashboard changes. Those live in the telemetry repo; quickstart.md gives the queries. | +| Personal edition. | Server edition. Its onboarding, starter and summary surfaces stay as they are. The REST additions are available in both editions under the existing activity auth rules. | + +## Definitions + +- **Fresh install**: the persisted onboarding state was never engaged (`OnboardingState.engaged == false`). This covers new installs and upgraded installs that never dismissed the wizard. +- **Install anchor**: a persisted timestamp written once, on the first core start that finds no onboarding record (`OnboardingState.install_first_run_at`). It never rotates; the telemetry `anonymous_id` rotates yearly. Installs upgraded from a build without the field have no anchor. Day-0 metrics are then reported as unavailable (`""`), never guessed. +- **Day 0**: the first 24 hours after the install anchor. +- **Indexed tool**: a tool document in the shared search index. A held (pending/changed) or quarantined tool is not indexed. The sticky flag of FR-023 is stamped when an index commit for an upstream server leaves at least one document. It is never stamped from a cached or fallback tool count. +- **Held tool**: a tool approval record in `pending` or `changed` state, or any captured tool of a quarantined server. +- **Starter candidate**: an entry in a short, fixed, compiled-in allowlist. Each entry names a catalog reference server that was audited against the starter criteria (FR-002), together with the expected tool set recorded at audit time. The list is ordered. The **selected starter** is the first candidate that passes all checks: + - its launcher (`uvx` or `npx`) resolves on the PATH the stdio spawner uses; + - its name is not taken; + - the **effective** `reference` registry is still the compiled `builtin/reference` source, and the catalog entry resolves to exactly the candidate's audited command. + + A config entry can override the `reference` registry by ID (`internal/registries/registry_data.go:9-14`). The check fails closed in that case. +- **Detected client**: a Spec 075 client registry entry that is `supported`, whose config location exists on this machine (`exists == true` in `GET /api/v1/connect`), and that is not currently in `connected_client_ids`. +- **Observed client**: a registry client ID that appeared in `connected_client_ids` at any observation. Observations happen on every onboarding read, every connect or disconnect action, and every heartbeat build, starting at the first start of a build with this feature. On an upgraded install, that first start records every currently connected client. The first-observed time per ID is persisted once and never overwritten. Observation does not depend on the install anchor, which is used only to bucket day-0 metrics. This covers clients connected outside the wizard and by hand-edited configs. +- **Brokered call**: an upstream `tool_call` that MCPProxy actually dispatched to an upstream server. It succeeded, or it failed after dispatch. A call refused before dispatch, for example by quarantine, a profile, a stale connection generation or the limiter, is never brokered. +- **Summary window**: the last 7 × 24 hours ending at request time. If the install is younger, the window starts at the install anchor. +- **Coverage watermark**: a persisted "activity log complete since T" timestamp. Every pruning pass (age, record count, byte size) advances it in the same transaction as the delete. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1: First run ends with a working tool (Priority: P1) + +A developer installs MCPProxy, opens the Web UI, connects Claude Code in the wizard and reaches the Servers step without having added a server. The step shows a pre-selected starter card: + +> Start with **time**: tells your agent the current time in any timezone. No account or API key needed. MCPProxy will download and run `uvx mcp-server-time` on this machine to list its tools; nothing can call them until you approve. + +They click "Add and inspect". MCPProxy adds the server through the normal catalog door, so it arrives quarantined like any catalog add, connects it for inspection and shows the captured tools with their risk tiers. They click "Approve and enable". The Verify step then says "1 server, 2 tools ready" and suggests a first prompt for the connected client ("Ask Claude Code: what time is it in Tokyo?"). + +**Why this priority**: tools indexed is the funnel step most strongly linked to one-and-done (36% vs 66%). It is the experiment's primary metric. + +**Independent Test**: on a scratch instance with an empty config and `uv` on PATH, complete the wizard with defaults. The summed `tool_count` across `GET /api/v1/servers` is at least 1. The activity log has the user's approval of `time`. No approval record has `approved_by` of `auto-baseline`. + +**Acceptance Scenarios**: + +1. **Given** a fresh install with zero indexed tools and `uvx` resolvable, **When** the user reaches the Servers step, **Then** the starter card for `time` is shown and pre-selected. It shows the exact command it will run, the isolation mode (host process, or Docker when isolation applies), "no credentials needed", and the expected tools. +2. **Given** the card is selected, **When** the user clicks "Add and inspect", **Then** the server is added through the catalog door, quarantined under the default config. The step shows "Connecting…" until definitions are captured (at most 90 s), then lists the **captured** tools with tiers and an "Approve and enable" button. +3. **Given** the captured review, **When** the user clicks "Approve and enable", **Then** the approval is sent bound to the shown definitions and the server incarnation (Spec 117 `expected_hashes` and `expected_incarnation`). The server leaves quarantine, its tools are indexed, and the Verify step reports the indexed tool count. +4. **Given** the captured tool set differs from the candidate's audited tool set (a tool added, removed or renamed by a newer package release), **Then** the step drops the starter copy ("tells your agent the time…") and shows a notice: "These tools differ from the version we reviewed." This is the normal review, with no starter framing. The approval still requires the same click. +5. **Given** the user unchecks the card or clicks "Skip", **When** they continue, **Then** nothing is added or run. The decline is remembered, and the wizard never pre-selects the starter again. +6. **Given** the user closes the wizard while the card is selected, before clicking "Add and inspect", **Then** nothing is added or run. +7. **Given** a definition changes between capture and approval, **When** the user clicks "Approve and enable", **Then** the 409 `review_out_of_date` response reloads the tool list and nothing is approved. This is Spec 117 behaviour, surfaced in place. +8. **Given** `quarantine_enabled: false`, or the default `scan` trust mode with a clean admission scan (Spec 119), **When** the starter is added, **Then** its tools are indexed without an approval step, exactly as for any other catalog add, and the step skips straight to "ready". +9. **Given** no candidate's launcher resolves (neither `uvx` nor `npx`), **Then** the card is replaced with "To try a starter server, install uv or Node.js", with links. The eligibility result is recorded as `launcher_missing`. + +--- + +### User Story 2: Users who skipped the wizard still get one path to a working tool (Priority: P2) + +A user dismissed the wizard, or configured a server that failed, and now has zero indexed tools. Home's "Get started" card, and a new empty-state card on the native macOS Home, offer the same starter, not pre-selected, with the same add → inspect → approve flow. + +**Why this priority**: the wizard's connect step wrote a client config for only 55% of the Aug-Oct cohort (`connected_client_ids` non-empty), so many installs leave the wizard early or never see it. macOS tray users may also never open the Web UI. + +**Independent Test**: with an engaged onboarding state and zero indexed tools, Home shows the starter offer. Accepting it from Home reaches the indexed state through the same review. + +**Acceptance Scenarios**: + +1. **Given** zero indexed tools, no held tools awaiting review and the starter not yet configured, **When** Home loads (Web or native), **Then** the starter offer is shown with an explicit "Add and inspect time" button, the command and isolation mode. Nothing is pre-selected. +2. **Given** the install has a server whose tools are held for review, **Then** Home shows the existing review call-to-action ("Review N tools from to start using them") instead of the starter offer. +3. **Given** the user previously declined the starter, **Then** Home shows the offer as a compact link rather than a card. A decline only stops pre-selection. +4. **Given** the native app, **When** the user accepts, **Then** the review step opens in the native review screen, and the approval is bound in the same way. + +--- + +### User Story 3: Connect a second client right after the first (Priority: P2) + +Once a client is connected, the wizard's Clients step shows "Connect another client" and lists the other agents it detected on this machine (Codex, OpenCode, Cursor…). Each row uses the existing preview-then-write flow, with the diff and backup notice shown before the Connect button. "Done" moves on. + +**Why this priority**: half of active machines run 2+ clients. A shared tool hub across agents is the product's core pitch, and a second connected client adds a second daily touchpoint. + +**Independent Test**: on a machine with Claude Code and Codex configs present, connect Claude Code in the wizard. Codex appears under "Connect another client". Connecting it adds `codex` to `connected_client_ids`, and the onboarding state records `second_client_connected = true`. + +**Acceptance Scenarios**: + +1. **Given** at least one client connected and at least one other detected client, **Then** the Clients step shows "Connect another client" with the detected clients in the registry's fixed order. Each has a Connect action that opens the existing preview. +2. **Given** no other detected client, **Then** the section shows one line, "Using another agent? See all clients", linking to the Clients hub, and `second_client_none_detected` is recorded. +3. **Given** the user clicks "Done" or moves to another tab without connecting a second client, **Then** `second_client_skipped` is recorded. +4. **Given** a client the access check reports as `denied` (Spec 075), **Then** its row shows the remediation instead of a Connect action. +5. **Given** the native Connect Client sheet, **When** a connect succeeds, **Then** the success state offers the same list ("Connect another client"), built from the same core payload. +6. **Given** the user skipped, or no other client was detected, **When** a second distinct client is later observed (from any surface, or by hand-editing a config), **Then** `second_client_connected` becomes true. Connected dominates the other facts, and nothing is ever cleared. +7. **Given** two clients were already connected before the wizard opened, **Then** `second_client_connected` is true. `second_client_offered` reflects only whether the section was shown. + +--- + +### User Story 4: A weekly reason to open MCPProxy (Priority: P3) + +Once a week, a user glances at the tray, or opens the Web UI, and sees what MCPProxy did for them: +- "This week: 1,284 calls brokered (Claude Code 812, Codex 431, other 41)"; +- "3 tool changes held for review"; +- "5 calls blocked (4 quarantine, 1 profile)"; +- "2 calls with secrets detected". + +Each figure links to the filtered Activity or Review view. + +**Why this priority**: it is the habit part of the experiment and the least certain lever. It is independent of US1-US3. + +**Independent Test**: seed an activity log with known records over 10 days. The Web card, the tray line, `mcpproxy activity summary --since 7d -o json` and `GET /api/v1/activity/summary?period=7d` report identical figures for the 7-day window. + +**Acceptance Scenarios**: + +1. **Given** brokered calls from two clients in the window, **Then** "calls brokered" equals the number of brokered calls (Definitions), split by client display name (top 3 plus "other"). Records with no client are counted as "unknown client". +2. **Given** a call refused before dispatch that left both a `tool_call` error record and a `policy_decision` blocked record for the same request, **Then** it counts once, under "blocked", and not under "calls brokered". +3. **Given** two tools that went pending and one approved tool that went changed in the window, **Then** "held for review" reports 3 distinct `(server, tool)` pairs. A tool held before the window and changed again inside it is not counted. A tool that flips twice counts once in the total. The per-kind breakdown (new / changed) may then sum to more than the total, and the response says so. The number currently awaiting review is shown next to it, with a link to the Review queue. +4. **Given** blocked or rejected calls, **Then** "blocked" reports the total and a split by reason class: `quarantine`, `profile` or `other`. Each refused attempt counts once. +5. **Given** brokered calls carrying sensitive-data detections, **Then** "sensitive data" reports how many calls had at least one detection, with a link to Activity filtered `sensitive_data=true`. +6. **Given** an install 3 days old, **Then** the summary is labelled "since (3 days)" rather than "this week". +7. **Given** all counts are zero, **Then** the card says "Quiet week" with one next step (connect a client, or try the suggested first prompt), never an empty grid. +8. **Given** the coverage watermark is later than the window start, because pruning removed records inside the window, **Then** the summary is marked `partial` and states the date the log is complete from. +9. **Given** an agent-token caller scoped to servers A and B, and held tools on server C, **Then** C's holds do not affect any figure in that caller's response. + +--- + +### User Story 5: The maintainer can judge E3 from telemetry (Priority: P1, ships with US1) + +The maintainer queries the telemetry store for post-release cohorts. Without any new identifier, for each install they can tell: +- whether the starter was eligible, offered, added and made live, or failed and why; +- whether a second client was offered, skipped or connected; +- how many distinct clients were observed on day 0, including for installs that stopped before day 0 ended; +- whether a tool was ever indexed, and how soon; +- whether the summary was viewed. + +**Why this priority**: without it the experiment cannot be judged or killed. + +**Independent Test**: schema-17 payload tests assert the new fields' presence, enum closure, bucket edges and stickiness. The PII scan rejects unknown keys and values in every new object. + +**Acceptance Scenarios**: + +1. **Given** a starter accepted and approved, **Then** later heartbeats carry `day_two.starter_live = true` and `day_two.starter_kind = "time"`, and `activation.first_tool_indexed_ever = true` from then on, even if the tool count later drops to 0. +2. **Given** a heartbeat built while the index is still loading at startup, **Then** `first_tool_indexed_ever` still reflects the persisted history, while `tool_count` keeps today's sampled semantics. +3. **Given** two clients observed 1 hour after the install anchor and the core stopping 2 hours after it, **Then** the heartbeats sent before the stop carry `clients_day0 = "2"` with `clients_day0_final = false`. +4. **Given** three clients observed within day 0 and a fourth on day 3, **Then** heartbeats after day 0 carry `clients_day0 = "3_plus"` and `clients_day0_final = true`, and the value never changes again. +5. **Given** telemetry is disabled, **Then** nothing new is sent. The local onboarding state still records every fact, so the UI behaves the same. + +--- + +### Edge Cases + +- The starter is already configured, under any name, with the same command and args. The offer is not shown. +- A different server already uses the candidate's name (a user's own `time`). That candidate is skipped with reason `name_taken` and the next one is tried. The starter never renames or overwrites a user server. +- The user removes the starter later. The offer may reappear on Home only, as a compact link. The sticky telemetry facts stay true. +- Capture times out after 90 s, for example because the first `uvx` download is slow or the machine is offline. The step shows "Still starting… Retry / Remove" and records `starter_last_failure = capture_timeout`. The server stays configured and quarantined, and nothing is approved. +- The starter connects but reports zero tools. This is treated as `connect_failed`, and the step offers Remove. +- The GUI-launched core has a reduced PATH (macOS Finder launch). Launcher resolution searches the same login-shell PATH that stdio spawning uses, so the offer never promises a launcher the spawner cannot find. +- Docker isolation is enabled globally. The starter follows the normal isolation rules. The card says "runs in Docker", and the capture timeout is 180 s instead of 90 s. +- A config `registries` entry with ID `reference` overrides the built-in source. If the effective source is no longer `builtin/reference`, or `reference/time` resolves to a command other than the audited one, that candidate fails closed with reason `reference_overridden`, and the next candidate is checked the same way. +- The resolved command changes between showing the card and clicking "Add and inspect" (a config hot-reload). The add re-resolves the entry. If the command differs from the one shown, nothing is added and the card re-renders with the new state, so consent must be given again. +- The wizard opens with a client already connected. The second-client section is shown immediately. +- The activity log holds 100k records. The summary must still answer within the SC-006 budget. +- Clock skew or a timezone change does not matter: windows are computed in UTC from record timestamps, and display uses local time. +- A tool approval goes pending only through a toggle that synthesises a record. It is counted in the window only if that path writes a record with `entered_hold = true` (FR-019 requires every hold entry to do so). It always counts in "currently awaiting". +- Sensitive-data detection runs asynchronously, so a call made seconds before the request may not yet show its detection. This is acceptable; there is no consistency guarantee within a minute. +- Server edition: the starter and second-client surfaces are not rendered, and `day_two` is omitted from the payload. + +## Requirements *(mandatory)* + +### Functional Requirements: starter (US1, US2) + +- **FR-001**: The system MUST offer a starter server in the Web wizard's Servers step when all of these hold: + - the install is fresh; + - zero tools are indexed; + - no held tools await review; + - no configured server matches a starter candidate's command; + - a candidate passes the launcher and name checks. + + The offer MUST be pre-selected unless the user previously declined it. Pre-selection covers only the add-and-inspect action, never an approval. +- **FR-002**: Starter candidates MUST come from a compiled-in allowlist. Each entry MUST be audited by a maintainer against these criteria: no required inputs (credentials, env or headers), launched as stdio by `uvx` or `npx -y`, no Docker needed, and, in the audited version, no tool that reads files, environment variables or arbitrary network resources. Each entry MUST record its audited package version and expected tool names. A unit test MUST check what is checkable statically: the entry exists in the reference catalog, it declares no required inputs, its launcher is `uvx` or `npx`, and it is not on the exclusion list. The safety claim is limited to the audited version. At runtime the captured inventory is authoritative (US1 scenario 4). + + The v1 list, in order: + 1. `reference/time` (`uvx mcp-server-time`; expected tools `get_current_time`, `convert_time`); + 2. `reference/sequentialthinking` (`npx -y @modelcontextprotocol/server-sequential-thinking`; expected tool `sequentialthinking`). + + `fetch` (arbitrary URL fetching), `everything` (it ships `get-env`, which returns all environment variables, plus LLM-sampling test tools), `filesystem`, `memory` and `git` MUST NOT be candidates. +- **FR-003**: Accepting the offer MUST add the server through the same catalog add door as a normal catalog add, with no request fields beyond what that door already accepts. Immediately before the add, the core MUST re-resolve the catalog entry and abort, adding nothing, if the resolved command differs from the command shown to the user (Definitions: starter candidate). Quarantine default, trust mode (whatever the door resolves; `scan` by default since Spec 119), inspection exemption, admission scan and definition capture MUST be applied exactly as for that door. The starter MUST NOT pass any quarantine, trust-mode or approval override. +- **FR-004**: Before the add, the offer MUST show the exact command, the isolation mode and the statement that it will be downloaded and run for inspection. The button MUST name that action ("Add and inspect"), not a generic Continue. +- **FR-005**: When the added starter is quarantined, the offer surface MUST show the **captured** tools with their review tiers. It MUST require an explicit "Approve and enable" action, bound to the shown definitions and the server incarnation (Spec 117). No approval MUST happen without that click. If the captured tool names differ from the audited set, the starter copy MUST be replaced by the plain review with a "differs from the reviewed version" notice. +- **FR-006**: The surface MUST skip the review and go straight to the ready state only when the unchanged catalog door produced an unquarantined server. Today that happens in two cases: quarantine is globally off, or under Spec 119's default `scan` trust mode the admission scan settles clean and the existing scan-gate path unquarantines the server. Under `manual`, or after a non-clean verdict, the in-place bound approval (FR-005) applies. Either way the starter adds no path of its own. +- **FR-007**: Closing the wizard, unchecking the card or clicking Skip MUST add and run nothing. Skip and uncheck MUST persist `starter_declined` in the onboarding state. +- **FR-008**: The Web Home "Get started" card and a new native Home empty-state card MUST offer the starter, never pre-selected, when zero tools are indexed, no held tools await review and no starter is configured. If held tools await review, they MUST show the review call-to-action instead. +- **FR-009**: Launcher resolution MUST search the PATH that stdio spawning effectively uses: the login-shell PATH (`shellwrap.LoginShellPATH`) plus the process PATH. It MUST NOT spawn the launcher. Results MAY be cached for 60 s. +- **FR-010**: Capture MUST time out after 90 s, or 180 s when Docker isolation applies to the starter. On timeout or failure, the surface MUST offer Retry and Remove and MUST record the failure category. Remove MUST use the normal server-removal path. +- **FR-011**: After the starter becomes live, the wizard's Verify step MUST show the indexed tool count and one suggested first prompt per connected client. The prompt text MUST be fixed per starter kind. +- **FR-012**: The core MUST expose the offer decision through one read endpoint: eligibility and reason, the selected candidate with command, isolation mode and audited tool names, the pre-select flag, and the starter facts. Web and native MUST NOT re-implement the eligibility rules. The core MUST record starter facts, the add included, through the onboarding state, whichever surface acted. + +### Functional Requirements: second client (US3) + +- **FR-013**: Once at least one client is connected, the wizard's Clients step MUST show "Connect another client" listing every detected client in the registry's fixed order. This applies whether the client was connected in this session, earlier or outside the wizard. +- **FR-014**: Each listed client MUST use the existing preview → write → undo flow. The diff and backup notice MUST be visible before the write button. No client config MUST be written without a per-client click. +- **FR-015**: A client whose on-demand access check is `denied` MUST show the Spec 075 remediation instead of a Connect action. `malformed` MUST show the existing malformed-config message. +- **FR-016**: The onboarding state MUST record four independent sticky booleans, set from false to true and never cleared: + - `second_client_offered`: the section was rendered with at least one detected client; + - `second_client_none_detected`: the section was rendered with no detected client; + - `second_client_skipped`: the user left the section without connecting a second client; + - `second_client_connected`: two or more distinct observed clients, on any install, upgraded ones included (no anchor needed). + + It MUST also record each observed client's first-observed time, written once. +- **FR-017**: The native Connect Client sheet's success state MUST offer "Connect another client" from the same core list. Client observation MUST happen in the core (Definitions: observed client), so every fact is recorded no matter which surface or tool connected the client. + +### Functional Requirements: weekly summary (US4) + +- **FR-018**: The activity summary MUST gain an additive `digest` section for any period: + + | Field | Contents | + |---|---| + | `calls_brokered` | Total brokered calls, plus per-client counts (top 3 + other + unknown) | + | `held_for_review` | Distinct `(server, tool)` pairs that entered pending or changed in the window, with per-kind counts; `currently_awaiting` from the review queue | + | `blocked` | Total refused-before-dispatch attempts, including `rejected`, each counted once, plus per reason class `quarantine`, `profile`, `other`; separately `responses_blocked` (post-dispatch output blocks, whose calls stay brokered and are not in the total) | + | `sensitive_data` | Brokered calls with at least one detection | + | `coverage` | `complete` or `partial`, plus `complete_since` (the coverage watermark) | + + Existing response fields MUST be unchanged. +- **FR-019**: "Entered pending or changed" MUST count actual entries into a held state. Every `tool_quarantine_change` record MUST carry two new metadata fields: + - `entered_hold`: true only when the record's status moved from not-held (none, approved, or approved-and-disabled) to `pending` or `changed`, **and** the hold is in effect, meaning quarantine is enabled for the server; + - `hold_kind`: `new` or `changed`. + + The digest counts distinct `(server, tool)` pairs with `entered_hold = true` in the window. A further change to an already-held tool (`refreshHeldContractSnapshot`) and a change recorded while quarantine is off never count. Records written before this field existed cannot be classified: until the window contains only new-format records, `held_for_review` carries `coverage: partial`. The digest MUST NOT reconstruct history from current approval state. Fixtures: an already-held tool changing inside the window (not counted), a change with quarantine off (not counted), a tool that enters, is approved and re-enters (counted once). +- **FR-020**: Reason classes MUST come from a fixed mapping table (see plan). `block_reason` values with the `profile_` prefix and the profile-scope refusal map to `profile`. Server-quarantine, tool-pending and tool-changed refusals map to `quarantine`. Everything else maps to `other`. Refusal sites that today write an untyped reason MUST persist a typed reason so that the mapping never matches on free text. +- **FR-021**: Every pruning mechanism (age, record count, byte size) MUST advance the coverage watermark in the same storage transaction as its delete. On the first start of a build with this feature, the watermark MUST be initialised to the earliest surviving record's timestamp. This is conservative: an earlier idle period may then show as partial. +- **FR-022**: All digest figures MUST be computed locally. No digest figure MUST leave the machine. Telemetry carries only view counts (FR-028). +- **FR-023**: Authorization MUST follow the existing summary rules, extended to every digest figure: + - for a scoped caller, every figure is computed only over servers in the caller's allowed set, including `held_for_review` and `currently_awaiting`; + - when an attribution filter is set (`profile`, `client`, `token`), the server-scoped review figures (`held_for_review`, `currently_awaiting`) MUST be returned as `null` with `reason: "not_attributable"`, because approvals carry no client or profile attribution. + + Tests MUST prove that holds on a disallowed server cannot change a scoped caller's response. +- **FR-024**: The Web Home MUST show a "This week" card with the four figures. Each links to its view: + - Activity filtered by the window; + - the Review queue; + - Activity filtered `status=blocked`; + - Activity filtered `sensitive_data=true`. + + It MUST follow the window label rules (US4 scenarios 6-8). +- **FR-025**: The tray glance block MUST show one line, "This week: N calls · H held · B blocked", that opens the native Activity view. The line MUST be hidden when the core is unreachable. +- **FR-026**: The CLI MUST accept `mcpproxy activity summary --since 7d` as an alias of `--period 7d`, with the same values. The table output MUST print the digest, and `-o json|yaml` MUST include it additively. + +### Functional Requirements: telemetry (US5) + +- **FR-027**: The heartbeat MUST bump to schema 17 and add: + - `activation.first_tool_indexed_ever`: bool, sticky and persisted, stamped per the Definitions; + - `activation.first_tool_indexed_after`: enum `"" | lt_10m | 10m_1h | 1h_24h | 1d_7d | gt_7d`, measured from the install anchor and set once; `""` when there is no anchor. +- **FR-028**: The heartbeat MUST add a `day_two` object, in the personal edition only. It contains only booleans, closed enums and buckets: + + | Field | Values | + |---|---| + | `starter_eligibility` | Result of the first eligibility check on a fresh install: `"" | eligible | launcher_missing | name_taken | reference_overridden | already_configured | tools_present | held_tools | not_fresh` | + | `starter_launcher` | Launcher availability, evaluated on every fresh-install eligibility check whatever the other outcomes: `"" (never checked) | uvx | npx_only | none` | + | `starter_offered`, `starter_added`, `starter_live`, `starter_declined` | bool, sticky | + | `starter_last_failure` | `"" | connect_failed | capture_timeout | approval_abandoned` | + | `starter_kind` | `"" | time | sequentialthinking` (kind of the first add) | + | `starter_surface` | `"" | wizard | home_web | home_native` (surface of the first add) | + | `second_client_offered`, `second_client_none_detected`, `second_client_skipped`, `second_client_connected` | bool, sticky | + | `clients_day0` | `"" | 0 | 1 | 2 | 3_plus`: distinct observed clients whose first observation falls in day 0; provisional during day 0, `""` without an anchor | + | `clients_day0_final` | bool, true once day 0 has ended (the value is then frozen) | + | `summary_views` | `{web, tray, cli}`, each `0 | 1 | 2_5 | 6_plus`: views since the last accepted send, the same reset-on-accept window as other delta counters, kept in memory (a restart loses unsent views) | + +- **FR-029**: The new fields MUST NOT carry server names, tool names, paths, timestamps, free text or new identifiers. `ScanForPII` MUST reject unknown keys inside `day_two` and `summary_views`, out-of-enum values, non-boolean flags and any serialized map or timestamp in these objects, following the closed-object precedent of `scanTrustModeDistribution`. The telemetry docs MUST list each field under "What is collected". +- **FR-030**: `approval_abandoned` MUST be recorded when a starter stays captured but unapproved for 24 h. It is an observation only and MUST NOT trigger any action. +- **FR-031**: The local onboarding state MUST record all facts whatever the telemetry opt-out setting. Opt-out gates only sending. +- **FR-032**: A summary view MUST be counted in the core when a summary request carries `view=web|tray|cli`, at most once per surface per hour. Surfaces MUST pass `view` only for user-visible renders, never for background refreshes. + +### Key Entities + +- **StarterCandidate**: compiled in. `{kind, catalog_id, launcher, audited_version, expected_tools[], first_prompt}`. +- **StarterOffer**: derived per request. `{eligible, reason, candidate{kind, command, isolation, expected_tools}, preselect, facts}`. +- **OnboardingState additions**: persisted in the existing bbolt record, all `omitempty`: + - `install_first_run_at`; + - starter facts: `starter_eligibility`, `starter_offered`, `starter_added`, `starter_live`, `starter_declined`, `starter_last_failure`, `starter_kind`, `starter_surface`, `starter_added_at`, `starter_server_name`; + - second-client facts: the four second-client booleans; + - `client_first_observed_at{client_id → time}`. +- **ActivationState addition**: `first_tool_indexed_ever` and `first_tool_indexed_at` in the existing `activation` bucket. +- **CoverageWatermark**: one persisted timestamp next to the activity bucket. +- **ActivityDigest**: derived. The additive summary section of FR-018. +- **SummaryViewCounter**: an in-memory delta counter per surface, with per-surface last-counted time for the hourly debounce. Drained on an accepted send. + +## Success Criteria *(mandatory)* + +Cohort definition for SC-001 to SC-004: +- non-CI installs per the dashboard CI filter; +- personal edition; +- continuity identity key (`cont`), with the IP key reported alongside; +- first seen on a build that includes this feature, with at least 14 days of observation. + +The release-to-release comparison is pre/post. It is associative, not causal: release composition, autostart behaviour and heartbeat availability are confounds and are reported with the result. + +### Measurable Outcomes + +- **SC-001 (primary)**: at least **82%** of the cohort have `tool_count > 0` on at least one heartbeat. This is the same heartbeat-sampled instrument as the 71.5% Aug-Oct 2026 baseline. The sticky `first_tool_indexed_ever` rate is reported separately and never substituted. A movement counts as real if it exceeds the larger of 3 points and the 95% interval of the previous four monthly cohorts. +- **SC-002 (primary)**: day-7 return on the IP key is **at least 18%** for the cohort, against 14.9% pooled Jun-Sep. The machine key is reported alongside, with the same noise rule as SC-001. +- **SC-003 (diagnostic)**: + - Launcher availability: `starter_launcher = none` is below 15% of installs with a non-empty `starter_launcher`. That field is evaluated independently of every other eligibility outcome, so `tools_present` or `name_taken` installs never dilute it. Above 15%, follow-up O1 (an embedded starter) is triggered. + - Conversion: at least 60% of installs with `starter_offered` reach `starter_live`. +- **SC-004 (diagnostic)**: the share of cohort installs with at least one connected client whose day-0 heartbeats show at least two distinct ids rises by at least 8 points over the pre-release baseline. Both sides use the same instrument: the union of `connected_client_ids` over heartbeats received within 26 h of the identity's first heartbeat. Today that is the startup heartbeat at about 5 minutes and the first daily one. The baseline uses pre-release cohorts (schema 16 or older); the comparison uses schema-17 cohorts. The richer `clients_day0` / `clients_day0_final` fields are a separate diagnostic with no pre-release baseline. Their first release cohort becomes the baseline for later releases. +- **SC-005 (safety)**: no code path makes a starter tool callable without an explicit user approval when a normal catalog add of the same server would be quarantined. A test asserts this for the wizard, Web Home and native doors. No starter code runs before the user clicks "Add and inspect". +- **SC-006 (performance)**: the 7-day summary with digest returns in under 1.5 s on a 100k-record activity log, measured with a new benchmark next to the summary handler tests. The Home card renders its figures within 2 s of Home load. +- **SC-007 (privacy)**: the schema-17 privacy tests pass. There are no new identifier fields. Every new field is enum-closed, and unknown keys are rejected. +- **Kill rule**: if SC-001 and SC-002 show no movement beyond the noise rule after two releases and four weeks of cohort data, the starter pre-selection and the summary card revert to opt-in. The telemetry stays. + +## Assumptions + +- **A1**: The starter is `time` when `uvx` resolves, otherwise `sequentialthinking` when `npx` resolves. `time` is preferred because it is useful in a real prompt and makes no network calls after the first package download. The other reference servers are excluded (FR-002). +- **A2**: "Lands in quarantine-exempt trusted state only if the catalog marks it trusted": no catalog source marks a server quarantine-exempt today (MCP-1072: provenance is informational), and this spec adds no such marker. Under Spec 119's `scan` default, a starter whose admission scan settles clean is unquarantined by the existing scan gate, exactly as any catalog add would be. Under `manual`, or after a non-clean verdict, the in-place review (FR-005) is what gets its tools indexed. The security model is unchanged in both cases. +- **A3**: Pre-selecting the add is acceptable only with informed consent. The add starts the 1-hour inspection exemption, which downloads and runs the package as a host process (Docker isolation is off by default) before any approval. This is true of every catalog add today. So the card states the command and isolation mode, the button names the action (FR-004), and nothing runs on close or Skip. Tool *calls* stay blocked until approval by the existing tool gate. +- **A4**: The experiment is judged pre/post on release cohorts, not with a randomised holdout. Weekly new installs (about 75-230, depending on the key) make a 10% holdout too small to read within the kill window. Cohort retention has been flat for four months, which gives a stable baseline. +- **A5**: Day-7 return is measured by heartbeat presence, which needs the core running. The summary helps D7 only through users who keep MCPProxy running or reopen it. This is why US4 is P3 and US1 carries the primary metric. +- **A6**: The summary reuses `GET /activity/summary` and `activity summary`, with an additive `digest`, a `--since` alias and a `view` parameter, instead of a new endpoint or command. That keeps one aggregation path. +- **A7**: "Calls brokered" counts dispatched upstream calls only, not internal built-in calls such as `retrieve_tools`. Users understand calls to their tools; built-ins are an implementation detail. +- **A8**: The second-client list uses `exists == true` from the stat-only connect list as "detected". It never opens a client config just to decide whether to list it (Spec 075's no-content-read rule). +- **A9**: Server edition is out of scope for the starter, second-client and Home card surfaces. The additive summary fields are edition-neutral. +- **A10**: Installs upgraded from an earlier build have no install anchor. Their day-0 and time-to-first-tool fields are `""`, and they are excluded from the SC-004 diagnostic. All other facts, including client observation and `second_client_connected`, work on upgraded installs. This is acceptable because the cohorts are new installs. + +## Open Questions (the design proceeds with the stated default) + +- **O1**: Embedded starter. If `launcher_missing` exceeds 15% (SC-003), add a compiled-in, in-process read-only starter that needs no runtime. Default: not in v1. It adds a new server kind with its own lifecycle verbs (see the in-process scanner trap, MCP-2396). +- **O2**: A weekly OS notification ("MCPProxy this week: …"). Default: not in v1. The tray line and Home card come first; a notification needs its own opt-in UX. +- **O3**: Should a decline also hide the Home offer entirely? Default: no. It becomes a compact link (US2 scenario 3). +- **O4**: Pin the audited starter version in the install command (`uvx mcp-server-time==X`) instead of relying on the captured-inventory check. Default: no. Pinning changes the shared reference catalog entry for every user and needs a release-time bump process. The captured inventory plus a bound approval already guarantees that the user approves what actually runs. + +## Out of Scope + +- Any change to quarantine, trust modes, scan policy or approval semantics. +- Telemetry receiver and dashboard changes. quickstart.md gives the queries only. +- A CLI starter flow. +- Server edition onboarding. +- Notifications and e-mail digests. + +## Commit Message Conventions *(mandatory)* + +Commits use `Related #` once an issue exists. `Closes #...` appears only in the PR that completes the work, with one keyword per issue. Commits carry no co-author trailers and no AI attribution. diff --git a/specs/123-day-two-activation/tasks.md b/specs/123-day-two-activation/tasks.md new file mode 100644 index 000000000..5a38be713 --- /dev/null +++ b/specs/123-day-two-activation/tasks.md @@ -0,0 +1,427 @@ +# Tasks: Day-Two Activation, Experiment E3 (Spec 123) + +**Input**: the design documents in `/specs/123-day-two-activation/` +**Prerequisites**: plan.md, spec.md, research.md (R1-R12), data-model.md, contracts/, quickstart.md + +**TDD rule (constitution V)**: write each `test` task first. It must FAIL for the stated behavioural reason on a build that compiles, before its paired `impl` task starts. Where a new symbol is needed, add a stub, so the failure is behavioural and not a compile error. Record the failing output in the PR description. + +**Format**: `[ID] [P?] [Story] Description`. +- `[P]`: the task can run in parallel with other `[P]` tasks in its group, because they touch different files. +- Stories: US1-US5 from spec.md. +- Phases map 1:1 onto plan.md D7 PRs (PR-A … PR-E). Each PR is cut from the then-current `origin/main`, gets the OpenCode Sol 6.1 review loop (10-round cap), and merges on its own, with no stacking. + +**Gates for every PR**: +- both golangci-lint v2 passes, bare and `--build-tags server`; +- `go test -race` for the touched packages; +- the internal/server lane under `-tags server` with the CI skip regex (CLAUDE.md); +- vitest when `frontend/` is touched, plus the Playwright sweep for `frontend/src/`; +- `./scripts/test-api-e2e.sh`, only after `pgrep -fl 'mcpproxy.*serve'` shows no other session's core. + +--- + +## Phase 1: Setup (PR-A branch `123-pr-a-measure`) + +- [ ] T001 Branch from `origin/main`. Record baseline timings (artifact: the PR-A description) for `go test -race ./internal/storage/... ./internal/telemetry/... ./internal/runtime/... ./internal/httpapi/...` in the PR description. +- [ ] T002 [P] Create the empty package `internal/onboarding/` (`doc.go` with the package contract: pure domain logic, no storage or HTTP imports). Add an import-boundary test, `internal/onboarding/imports_test.go`, that fails if the package imports `internal/storage`, `internal/httpapi` or `internal/server`. + +--- + +## Phase 2: User Story 5, measure first (P1; foundational, blocks Phases 3-6; PR-A) 🎯 ships first + +**Goal**: persist every local fact and send schema 17, with no visible UI change. This gives a release of baseline data (research R12). +**Independent test**: schema-17 payload tests, plus quickstart L4. + +### Install anchor and onboarding state + +- [ ] T003 [P] [US5] test: `internal/storage/onboarding_anchor_test.go`. + - (a) `EnsureInstallAnchor` on an empty store creates the record with `InstallFirstRunAt`. + - (b) On an existing record without the field, it leaves it zero (upgrade). + - (c) `UpdateOnboardingState` on a missing record sets the anchor (the safeguard). + - (d) Update-before-get keeps the anchor. + - (e) Two concurrent first writers produce one anchor; run under `-race`. + - (f) The anchor is never changed by later updates. +- [ ] T004 [US5] impl: the `internal/storage/models.go` `OnboardingState` additions (data-model §1, all `omitempty`). `EnsureInstallAnchor` and the missing-record branch go in `internal/storage/bbolt.go` (~1221-1305), with a `manager.go` wrapper. Call `EnsureInstallAnchor` in `internal/server/server.go` startup **before** the HTTP server, connect service and MCP initialisation start. +- [ ] T005 [P] [US5] test: `internal/onboarding/facts_test.go`. + - sticky boolean merge (true never reverts); + - `MergeObservedClients` adds unseen IDs only and never overwrites a time; + - `second_client_connected` at 2 entries; + - `clients_day0` buckets `0/1/2/3_plus` with the boundary at exactly +24h; + - `""` without an anchor; + - `clients_day0_final`; + - `FirstToolIndexedAfter` buckets at 10m/1h/24h/7d. +- [ ] T006 [US5] impl: `internal/onboarding/facts.go`, pure functions only. + +### Facts loop (telemetry-independent) + +- [ ] T007 [P] [US5] test: `internal/runtime/onboarding_facts_test.go`, using a fake connect service and the real index manager over a temp dir. + - (a) The startup tick records currently connected clients on an upgraded store. + - (b) It runs with telemetry disabled. + - (c) A successful `batchIndexTools` records index evidence `(server, incarnation)` at commit time, and the loop persists `first_tool_indexed_ever` with `first_tool_indexed_at` equal to the **commit** time. + - (d) Index commit, then disable/quarantine purge **before** the loop runs: the fact still lands (evidence retained). + - (d2) Saturate the size-1 wake channel, then run only unchanged discoveries: no evidence is lost. + - (d3) A persist failure keeps the evidence, and the next tick retries. + - (d4) Upgrade supplement: tools already indexed and no evidence → `ever=true`, `after=""`. + - (e) No storage write happens while the per-server approval lock is held. Assert via the `commit_testhooks.go` hooks. + - (f) `GetToolCount()` fallback values never stamp the flag (an empty index with `lastToolCount > 0` must not stamp). + - (g) Shutdown runs one bounded final reconcile. +- [ ] T008 [US5] impl: + - `internal/runtime/onboarding_facts.go`: the `factEvidence` sync.Map, `onboardingFactsLoop`, `reconcileActivationFacts` and `observeConnectedClients`. It does I/O outside the transaction, then one pure `UpdateOnboardingState` merge, and deletes evidence only after persisting it. + - Index-evidence producer plus non-blocking wake in `internal/runtime/absent_tool_removal.go` `batchIndexTools`, with the incarnation passed in by the `lifecycle.go` apply callers. Capture-evidence producer at review-capture completion (`lifecycle.go:~1066`). + - Start and stop wiring in `internal/server/server.go`. +- [ ] T009 [P] [US5] test: `internal/httpapi/onboarding_observe_test.go`. + - Connect and disconnect handlers and `GET /onboarding/state` call observation. + - The `POST /onboarding/mark` new booleans (`starter_offered`, `starter_declined`, `starter_failure`, `second_client_*`) are sticky. + - An invalid `starter_failure` value returns 400. + - Agent tokens are denied (the existing `requireAdminRead` rule). +- [ ] T010 [US5] impl: in `internal/httpapi/onboarding.go` and `internal/httpapi/connect.go`, add the mark fields, observation calls, and the `day_two` local-facts block in the state response (contracts/onboarding-starter.openapi.yaml). + +### Telemetry schema 17 + +- [ ] T011 [P] [US5] test: `internal/telemetry/activation_tool_indexed_test.go`. + - `MarkFirstToolIndexed` is sticky. + - `first_tool_indexed_at` is stored and **never** serialised. + - `first_tool_indexed_after` is derived correctly, and `""` without an anchor. + - Every fake `ActivationStore` in the repo compiles with the new method; grep the test tree. +- [ ] T012 [P] [US5] test: `internal/telemetry/payload_v17_test.go` (`TestPayloadV17`). + - `schema_version == 17`. + - The `day_two` keys match `contracts/telemetry-day-two.schema.json` exactly. + - Server edition omits `day_two`. + - Opt-out sends nothing. + - `summary_views` drains only on an accepted send. + - Log one sample payload (quickstart L4). +- [ ] T013 [P] [US5] test: `internal/telemetry/anonymity_day_two_test.go` (`TestScanForPII*DayTwo`). Rejections: + - an unknown `day_two` key; + - an unknown `summary_views` key; + - out-of-enum values for every enum field; + - a non-boolean flag; + - an RFC3339 timestamp or a map value inside `day_two`; + - a server-name-like string in `starter_kind`. + + The violation must not echo the offending key. Accept: a valid full object. +- [ ] T014 [US5] impl: + - `internal/telemetry/activation.go`: key, field, load, save, `MarkFirstToolIndexed`. + - `internal/telemetry/day_two.go`: `DayTwoPayload`, `SetDayTwoProvider`, summary-view counter with an atomic per-surface 1-hour debounce, bucketing. + - `internal/telemetry/telemetry.go`: `SchemaVersion = 17`, a change-log paragraph, payload mapping. + - `internal/telemetry/anonymity.go`: `scanDayTwo` plus the activation enum rule. + - Provider wiring in `internal/server/server.go`, next to `SetOnboardingProvider` (~3234). +- [ ] T015 [US5] Move every pin of the then-current schema (16 on `origin/main` at 2026-10-25) to the new version. List them with `grep -rnE 'schema_version.{0,3}16|want 16' internal/telemetry docs` and get no hits afterwards. If main has bumped again, use the next free version and update this spec. Keep `schema_version_guard_test.go` green. + +### Docs (PR-A) + +- [ ] T016 [P] [US5] Docs: + - `docs/features/telemetry.md`: schema 17, `day_two` and activation fields under "What is collected", privacy notes, the comparability note for `tool_count` versus `first_tool_indexed_ever`. + - `docs/features/connect-clients.md`: client observation. + - A `CLAUDE.md` "Recent Changes" line. +- [ ] T017 [US5] PR-A verification: + - quickstart L4; + - opt-out run (`MCPPROXY_TELEMETRY=false`) shows facts recorded locally (`GET /onboarding/state`); + - Sol 6.1 review loop; + - merge. + +**Checkpoint**: PR-A merged. Facts and schema 17 are live; no UI has changed. + +--- + +## Phase 3: User Story 1 + User Story 2 (Web): starter (PR-B branch `123-pr-b-starter`) 🎯 MVP + +**Goal**: a fresh install reaches indexed tools through add → inspect → bound approval, with the security model unchanged. +**Independent test**: quickstart L1 and L2 (steps 2-4). + +### Core + +- [ ] T018 [P] [US1] test: `internal/shellwrap/resolve_test.go`. + - `ResolveOnEffectivePATH` finds a binary present only on the login-shell PATH, using a fake `LoginShellPATH` seam. + - It falls back to the process PATH. + - It never executes the binary: assert with a sentinel script that would create a file. + - On Windows it uses the process PATH only (build-tagged). +- [ ] T019 [US1] impl: `ResolveOnEffectivePATH` in `internal/shellwrap/shellwrap.go`. +- [ ] T020 [P] [US1] test: `internal/onboarding/starter_test.go`. + - **Static audit (FR-002)**: each allowlist entry exists in `registries` reference data with the exact audited command, declares no required inputs, has launcher `uvx` or `npx`, is not in the exclusion list (`fetch`, `everything`, `filesystem`, `memory`, `git`), and has non-empty `expected_tools`, `audited_version` and `first_prompt`. + - **Selection**: + - `uvx` present → time; + - only `npx` → sequentialthinking; + - neither → `launcher_missing` with `starter_launcher = none`; + - name `time` taken → skip to the next (`name_taken`); + - `reference` provenance overridden → `reference_overridden`; + - same command already configured → `already_configured`; + - indexed tools → `tools_present`; + - held tools → `held_tools`; + - not fresh → no pre-selection; + - declined → no pre-selection; + - server edition → `server_edition`. + - `starter_launcher` is computed whatever the other reasons are (SC-003 denominator). +- [ ] T021 [US1] impl: `internal/onboarding/starter.go` (allowlist R1, `Select`) and `internal/onboarding/eligibility.go`. +- [ ] T022 [P] [US1] test: `internal/server/add_from_registry_split_test.go`. + - The public door's behaviour is unchanged: the existing add-from-registry tests pass unmodified. + - `resolveRegistryEntry` + `addResolvedRegistryEntry` produce the identical config. + - **Swap test**: a test hook swaps the `reference` registry between the starter's resolve and add. The configured command is the validated one, and no second resolution happens. +- [ ] T023 [US1] impl: split `internal/server/add_from_registry.go:88-141` into `resolveRegistryEntry` and `addResolvedRegistryEntry`. `AddServerFromRegistry` calls both. +- [ ] T024 [P] [US1] test: `internal/httpapi/onboarding_starter_test.go`. All tests are routed through the real router. + - GET is admin-read only. + - POST with an agent token → 403, with no resolution, add or fact write. + - A valid POST adds through the door (quarantined by default) and records `added`, `kind`, `surface`, `server_name` and the **create-commit** `incarnation` from `CommitResult`, never a post-add lookup. + - `shown_command` mismatch → 409 `starter_changed` with a fresh offer, nothing added. + - `duplicate_name` → 400. + - The POST never approves: the server stays quarantined and no `quarantine_change` approval record exists. + - `quarantine_enabled: false` → the tools get indexed with no approval call. + - Envelope `{success, data}`. +- [ ] T025 [US1] impl: + - `internal/server/onboarding_starter.go`: resolve once, validate the snapshot, `addResolvedRegistryEntry`, record facts, wake the facts loop. + - `internal/httpapi/onboarding.go`: routes with `requireAdminRead` and `requireServerOp(auth.ServerOpAddFromRegistry)`. + - The `contracts.StarterOffer` type in `internal/contracts/onboarding.go`. +- [ ] T026 [P] [US1] test: `internal/runtime/starter_live_test.go`. + - An approved starter's index evidence sets `starter_live`. + - Evidence produced **before** the POST records the starter is retained and matched afterwards. + - **Interleaving**: A is the recorded starter, A is removed, same-name B is created and indexed; B's evidence never sets live for A. Pause hooks are placed between the evidence copy and the transaction. + - `starter_captured_at` comes only from capture-completion evidence. A partial capture that fails in `checkToolApprovalsHeld` or `pruneAbsentUndecidedApprovals` yields no evidence and no clock. + - Abandonment: captured + 24h + unapproved → `approval_abandoned`. Never captured → it keeps `capture_timeout`. Live → never abandoned. This uses a fake clock. +- [ ] T027 [US1] impl: the starter branches of `reconcileActivationFacts` and the abandonment check (R4, R4a) in `internal/runtime/onboarding_facts.go`. +- [ ] T028 [P] [US1] test, SC-005 safety: `internal/server/starter_quarantine_safety_test.go`. + - With default config, the wizard, Home-web and Home-native surfaces (all three call the same POST) add the starter. + - A `call_tool_read time:get_current_time` is refused, and the stub upstream saw zero `tools/call`. + - Only a bound `security/approve` makes it callable. + - It runs in the internal/server race lane. + +### Web + +- [ ] T029 [P] [US1] test: `frontend/tests/unit/starter-offer-card.spec.ts`. + - The wizard mode is pre-selected and shows the command and isolation. + - The button reads "Add and inspect". + - Closing or Skip sends no POST. + - Skip sends `starter_declined`. + - The state machine runs idle → adding → capturing → review → approving → live. + - The approval call carries `expected_hashes` **and** `expected_incarnation` from the displayed review. + - 409 `review_out_of_date` reloads the review. + - 409 `starter_changed` re-renders the offer. + - A captured tool set different from `expected_tools` shows the "differs from the version we reviewed" notice and drops the starter copy. + - The capture timeout (90 s, or 180 s for docker) shows Retry/Remove and marks `capture_timeout`. + - Home mode is not pre-selected, and shows the compact link after a decline. +- [ ] T030 [US1] impl: + - `frontend/src/components/onboarding/StarterOfferCard.vue`; + - API functions in `frontend/src/services/api.ts` and types in `frontend/src/types/api.ts`; + - `frontend/src/stores/onboarding.ts`. +- [ ] T031 [P] [US1] test: `frontend/tests/unit/onboarding-wizard-starter.spec.ts`. + - The Servers tab shows the card when the offer is eligible. + - The tab is **not** complete while the starter is quarantined or uncaptured (`hasUsableServer` is unchanged). + - It completes after live. + - Verify shows the tool count and one first prompt per connected client. + - The existing `onboarding-wizard-servers-step.spec.ts` stays green. +- [ ] T032 [US1] impl: `OnboardingWizard.vue` Servers and Verify integration (lines ~202-470). +- [ ] T033 [P] [US2] test: extend `frontend/tests/unit/home-getting-started.spec.ts`. + - The offer shows on zero indexed tools. + - With held tools awaiting review, the review CTA shows instead. + - Not pre-selected. + - Compact link after a decline. +- [ ] T034 [US2] impl: the `Home.vue` "Get started" card (~17-42) hosts `StarterOfferCard` in home mode. +- [ ] T035 [US1] Docs: + - `docs/web-ui/dashboard.md`: Home and the wizard starter. + - `docs/features/connect-clients.md`: the first-run starter, with the security note "downloads and runs on Add and inspect; tools callable only after approval". + - `oas/swagger.yaml`: `/onboarding/starter`. + - A `CLAUDE.md` line. +- [ ] T036 [US1] PR-B verification: + - quickstart L1, including the negative checks and the MCP refused-call check (correlated `policy_decision`); not the PR-D dispatch metadata; + - quickstart L2 steps 2-4 only (step 1 is PR-C), Playwright, with screenshots; + - Sol loop; + - merge. + +**Checkpoint**: US1 and US2 (Web) are usable and measurable. + +--- + +## Phase 4: User Story 3 (Web): second client (PR-C branch `123-pr-c-second-client`) + +**Goal**: offer every detected, unconnected client right after the first one connects. +**Independent test**: quickstart L2 step 1. + +- [ ] T037 [P] [US3] test: `internal/httpapi/onboarding_detected_clients_test.go`. + - `detected_unconnected_clients` lists `supported && exists && !connected` in the registry's fixed order. + - It never reads client config content: the stat-only listing is spied (Spec 075). +- [ ] T038 [US3] impl: `detected_unconnected_clients` in the `internal/httpapi/onboarding.go` state response. +- [ ] T039 [P] [US3] test: `frontend/tests/unit/client-connect-list-per-client.spec.ts`. + - The `per-client` mode renders no bulk-connect footer or loop. + - Each row needs its own preview confirmation. + - The diff and backup notice show before the write button. +- [ ] T040 [US3] impl: the `per-client` mode prop on `frontend/src/components/ClientConnectList.vue` (hides `:470-486` and the bulk loop `:1275-1286`). +- [ ] T041 [P] [US3] test: `frontend/tests/unit/second-client-section.spec.ts`. + - It shows once at least one client is connected, including when the client was connected before the wizard opened. + - `second_client_offered` is marked on render with at least one detected client. + - `none_detected` shows a link to the Clients hub and its mark. + - Done or a tab change without a new connect marks `skipped`. + - A `denied` access state shows the remediation. + - `malformed` shows the existing message. + - **Server edition**: the section never renders (personal-edition guard, spec A9). +- [ ] T042 [US3] impl: `frontend/src/components/onboarding/SecondClientSection.vue` and its Clients-tab integration in `OnboardingWizard.vue`, with a personal-edition guard read from `/api/v1/status` `edition`. +- [ ] T043 [US3] Docs: `docs/features/connect-clients.md` gets "Connect another client". Add a `CLAUDE.md` line. Verify with the Playwright sweep, run the Sol loop, merge. + +**Checkpoint**: US3 (Web) is done. `second_client_connected` is already measured by PR-A on every surface. + +--- + +## Phase 5: User Story 4: weekly digest, core + Web + CLI (PR-D branch `123-pr-d-digest`) + +**Goal**: a local "what MCPProxy did" summary, identical on every surface. +**Independent test**: quickstart L3. + +### Activity instrumentation + +- [ ] T044 [P] [US4] test: `internal/server/tool_call_dispatched_test.go`. This table test drives every `tool_call` completion exit in `internal/server/mcp.go` and asserts `metadata.dispatched`: + - false for argument validation (`~3221`), missing or disconnected client (`~3246,~3319`), the review-not-recorded or generation refusal (`~3446-3475`), direct **and** sandbox limiter sheds (`RecordToolCallRejected`, `internal/runtime/activity_service.go:555-576`) and sandbox profile sub-call refusals (`mcp_code_execution.go:~1289-1299`); + - true for success, an upstream error, output sanitisation block (`~3566`) and the strict output-schema block (`~3610-3653`, which must now emit the canonical completion). + + It runs in the internal/server race lane. +- [ ] T045 [US4] impl: set `Metadata["dispatched"]` from the existing dispatch evidence (`noteUpstreamDispatched`, `callerr.Facts.Dispatched`) in the completion funnel, in `RecordToolCallRejected` and in sandbox sub-call refusals. Add the canonical completion on the strict output-schema exit. +- [ ] T046 [P] [US4] test: `internal/server/policy_decision_block_reason_test.go`. + - `emitActivityPolicyDecisionWithBlockReason` persists `reasonKey` as `BlockReason` when no profile reason is given. + - The pre- and post-dispatch keys are both persisted. + - The profile `block_reason` values are unchanged. + - **Direct vs sandbox parity**: server quarantine, pending approval and changed approval refusals produce the same `BlockReason` and the same `BlockClass` (`quarantine`) whether called directly or from `code_execution`. +- [ ] T047 [US4] impl: + - `internal/server/mcp.go` (~989-1009). + - `internal/server/mcp_code_execution.go` (~958-965): pass `policyRefusalReasonKey(gate)` to `emitSubCallRefused` for the sandbox quarantine and approval-lock refusals. + - Update `internal/profile/testdata/contract/{labels,refusals}.json` and the Activity view labels in `frontend/src/` for the new values. Run the profile contract tests and the Activity vitest. +- [ ] T048 [P] [US4] test: `internal/runtime/tool_quarantine_entered_hold_test.go`. + - (a) A table that counts every `emitToolQuarantineEvent` call site by grep, so it fails when a site is added without a decision. + - (b) A new pending record with quarantine on → `entered_hold=true`, `new`. + - (c) approved → changed with quarantine on → `true`, `changed`. + - (d) The same with quarantine off → `false`. + - (e) `refreshHeldContractSnapshot` → `false`. + - (f) `adoptLegacyLockOrBaseline` creating pending, in both the failed-scan and manual branches → `true`, `new`. Today it emits nothing. + - (g) legacy lock carry-over, including the `:2262` emit → `false`. + - (h) The toggle-synthesised pending → an additional `tool_discovered` with `true`. + - (i) Auto-approved, approved and blocked → `false`. +- [ ] T049 [US4] impl: the `enteredHold` and `holdKind` parameters and the new emits in `internal/runtime/tool_quarantine.go`. Metadata passthrough in `internal/runtime/activity_service.go` (~1389-1424). +- [ ] T050 [P] [US4] test: `internal/storage/activity_watermark_test.go`. + - Each of the three prune functions advances `activity_complete_since` to the newest deleted record + 1 ns, in the same transaction. Inject a failure after the delete; the watermark does not move. + - It never moves backwards. + - First open initialises it to the oldest surviving record, or to now for an empty log. + + Constraint: the first-open initialisation must not hold the activity bucket long on a 100k-record log; it reads only the first key. +- [ ] T051 [US4] impl: the watermark in `internal/storage/activity.go` (`:342`, `:393`, `:451`) and its initialisation. + +### Digest + +- [ ] T052 [P] [US4] test: `internal/storage/activity_digest_test.go` with `internal/storage/testdata/digest_fixture.json` and `internal/storage/testdata/digest_expected.json`. The fixture holds 10 days of records: + - two clients, plus calls with no client; + - a refusal pair (`tool_call` with `dispatched=false` plus a pre-dispatch blocked decision), counted once in blocked; + - an unpaired `dispatched=false` limiter shed → `blocked.other`; + - an unpaired sandbox profile refusal (`profile_tier`) → `blocked.profile`; + - output-sanitisation and strict-schema blocks → brokered + `responses_blocked` (paired by `request_id` with a dispatched `tool_call`); + - a blocked `prompts/get` output-sanitisation decision → zero contribution to every digest figure; + - limiter `rejected` → `other`; + - quarantine and profile blocks; + - holds: enter, approve, re-enter (counted once); an already-held change (not counted); a quarantine-off change (not counted); + - legacy records without `dispatched` or `entered_hold` → partial coverage for those figures; + - sensitive detections on brokered calls only; + - a watermark inside the window → `coverage: partial`. + + The test asserts the exact expected JSON. +- [ ] T053 [US4] impl: `internal/storage/activity_digest.go` with `DigestAccumulator`, `BlockClass` and an exported `SensitiveDataDetected` (it wraps `extractSensitiveDataInfo`, `activity_models.go:726`). +- [ ] T054 [P] [US4] test: `internal/httpapi/activity_digest_test.go`. + - `GET /activity/summary?period=7d` includes `data.digest`, and the existing fields are unchanged (the existing summary tests stay green). + - **Scope**: a scoped token on servers {a, b} gets a **byte-identical digest** with and without c's history. That covers holds on c (`held_for_review`, `currently_awaiting`), a prune of c's records and a watermark initialised from a c record. Scoped callers get `coverage.state = unavailable`. + - The `profile`, `client` and `token` filters give `held_for_review` null with `reason: not_attributable`. + - `view=web` counts once per hour, `view=bogus` is ignored without an error, and no `view` counts nothing. + - **Window**: install 3 days old → `window.label = since_install`, `days = 3`, `effective_start` = anchor; install 30 days old → `period`; anchorless upgrade → `period`. `start_time` is unchanged in all three. +- [ ] T055 [US4] impl: `internal/httpapi/activity.go` (~886-1051) feeds the accumulator in the existing stream loop. `Finish` uses `Runtime.GetReviewQueue` filtered by `AllowedServers` and adds the watermark. The `view` parameter calls `telemetry.RecordSummaryView`. Add `contracts.ActivityDigest` in `internal/contracts/activity.go`. +- [ ] T056 [P] [US4] bench: `internal/httpapi/activity_digest_bench_test.go`. A 100k-record log and a 7-day window, under 1.5 s (SC-006). Record the result in the PR. +- [ ] T057 [P] [US4] test: `cmd/mcpproxy/activity_summary_since_test.go`. + - `--since 7d` equals `--period 7d`. + - Invalid values give the same error as `--period`. + - The request carries `view=cli`. + - The table prints the digest block and the "since (N days)" or partial labels, taken from `digest.window` and `digest.coverage`. + - `-o json` includes `digest`. +- [ ] T058 [US4] impl: `cmd/mcpproxy/activity_cmd.go` (flags ~1314-1322, render ~2426-2490). +- [ ] T059 [P] [US4] test: `frontend/tests/unit/weekly-digest-card.spec.ts`. + - The four figures and their links: Activity window, Review queue, `status=blocked`, `sensitive_data=true`. + - "Quiet week" with one next step. + - The "since (N days)" label. + - The partial label. + - `view=web` only on a visible mount and never on SSE refresh. + - Null review figures under attribution filters render as "n/a". + - The label comes from `digest.window` (`period` → "This week", `since_install` → "since (N days)"); the component never computes it. + - **Server edition**: the card never renders (personal-edition guard, spec A9). +- [ ] T060 [US4] impl: `frontend/src/components/WeeklyDigestCard.vue` on `Home.vue`, between `AttentionList` and `UsageSummaryStrip`, behind a personal-edition guard. Update the API types. +- [ ] T061 [US4] Docs: + - `docs/cli/activity-commands.md`: `--since`, the digest table and JSON. + - `docs/features/activity-log.md`: `dispatched`, the typed block reasons, `entered_hold` and the watermark. + - `docs/web-ui/dashboard.md`: the "This week" card. + - `oas/swagger.yaml`: `view` and `digest`. + - A `CLAUDE.md` line. +- [ ] T062 [US4] PR-D verification: quickstart L3 (the REST and CLI outputs are identical), the Playwright sweep, the Sol loop, merge. + +**Checkpoint**: US4 core, Web and CLI are done. + +--- + +## Phase 6: Native macOS: US2, US3 and US4 native (PR-E branch `123-pr-e-native`) + +**Goal**: the same starter, second-client and weekly surfaces in the tray app, with bound approvals. +**Independent test**: quickstart L5. + +- [ ] T063 [P] [US2] test: `native/macos/MCPProxy/MCPProxyTests/ApproveIncarnationTests.swift`. + - `securityApproveServer` sends `expected_incarnation` taken from the decoded review. + - A remove and re-add before the approval surfaces 409 `server_replaced` as a reload. +- [ ] T064 [US2] impl: decode the incarnation in `API/Models.swift` (review model). Add the `expectedIncarnation` parameter to `API/APIClient.swift` `securityApproveServer` (~290-298) and pass it from the review screen. +- [ ] T065 [P] [US2] test: `MCPProxyTests/StarterOfferTests.swift`. + - The empty-state card shows when the offer is eligible, with the command and isolation. + - "Add and inspect" calls POST with `surface=home_native` and `shown_command`. + - It then opens the native review for the returned server. + - Not pre-selected. + - The held-tools state shows the review CTA instead. +- [ ] T066 [US2] impl: `Views/StarterOfferCard.swift` (new), the `Views/HomeView.swift` empty state, and the `APIClient` starter calls. +- [ ] T067 [P] [US2] test: `MCPProxyTests/StarterFlowModelTests.swift`. This covers the native starter state machine (idle → adding → capturing → review → approving → live, plus failed), with a fake API. + - Capture waiting runs 90 s, or 180 s for docker, then shows Retry/Remove and marks `capture_timeout`. + - A captured tool set different from `expected_tools` shows the "differs from the version we reviewed" notice. + - With quarantine off, it goes straight to ready (FR-006). + - **No blind approval in the starter flow**: the approve control stays disabled until the review is captured and non-empty (FR-005). The ordinary review screen's blind-approval path (`Views/ReviewQueueView.swift:244-254,384-411`) is unchanged; a regression test confirms that. + - 409 `review_out_of_date` reloads; 409 `starter_changed` re-renders the offer. +- [ ] T068 [US2] impl: `Views/StarterFlowModel.swift` (new) drives the card through capture → inline captured review → bound approve (hashes + incarnation, T064), and opens the full native review only on request. +- [ ] T069 [P] [US3] test: `MCPProxyTests/ConnectAnotherClientTests.swift`. The success state lists `detected_unconnected_clients`, each row opens the existing per-client connect flow, and with none detected there is a link to the Clients view. Uses `Support/FakeConnectSource.swift`. +- [ ] T070 [US3] impl: the `Views/ConnectClientView.swift` success state (`case .succeeded`, ~511-545) and `ConnectClientModel.swift`. +- [ ] T071 [P] [US3] test: `MCPProxyTests/SecondClientMarksTests.swift`. + - Rendering the success-state list with ≥1 detected client sends `second_client_offered`; with none it sends `second_client_none_detected`. + - Closing the sheet without connecting another client sends `second_client_skipped`. + - The marks are sent once (sticky). + - A later core-observed second client still yields `second_client_connected`, through the facts loop. +- [ ] T072 [US3] impl: add `markOnboarding(_:)` to the `ConnectClientModel` data-source seam (`ConnectClientModel.swift:11-59`) and to `APIClient` (`POST /api/v1/onboarding/mark`). Call it from the success state. +- [ ] T073 [P] [US4] test: `MCPProxyTests/WeeklyLineTests.swift`. + - The line format is "This week: N calls · H held · B blocked". + - It is hidden when the core is unreachable. + - `view=tray` is sent only from `menuWillOpen`. + - Clicking the line opens the native Activity view (FR-025). + - The label uses `digest.window`. + - `ActivitySummary` decodes both old payloads (fields absent) and new ones. +- [ ] T074 [US4] impl: + - `API/Models.swift` `ActivitySummary` catch-up (`call_count`, `call_error_count`, `rejected_count`, `other_count`, optional `digest`); + - `APIClient.activitySummary(period:view:)`; + - the `Menu/Glance/GlanceSection.swift` weekly line. +- [ ] T075 [US2] PR-E verification (covers US2, US3 and US4 native): quickstart L5 with `mcpproxy-ui-test` screenshots (tray dev-swap recipe), the Swift tests, the Sol loop, merge. + +--- + +## Phase 7: Polish and cross-cutting + +- [ ] T076 [P] After PR-B ships, a `mcpproxy doctor` hint when `starter_launcher = none` ("Install uv or Node.js to try starter servers"), in `cmd/mcpproxy/doctor_cmd.go`. Optional, and only if SC-003 shows `launcher_missing` above 5%. +- [ ] T077 After PR-B, PR-C and PR-D have all merged, run the combined verification: quickstart L1-L3 in full, including the PR-D dispatch metadata, the L2 steps 1-4 Playwright sweep, and server-edition negative checks for the starter, second-client section and weekly card. File follow-ups for any regression. +- [ ] T078 Two releases after PR-B, run the quickstart E3 queries (SC-001 to SC-004) and apply the kill rule (spec). Record the verdict in `mcpproxy-promotion` reports. This is maintainer analysis, not code. + +--- + +## Dependencies and execution order + +- **Phase 1 → Phase 2 (PR-A)** blocks every other phase. The facts, loop and telemetry are shared. +- **Phase 3 (PR-B)**, **Phase 4 (PR-C)** and **Phase 5 (PR-D)** depend only on PR-A, so they can be developed in parallel, in separate worktrees. They touch different files, except `OnboardingWizard.vue` (PR-B Servers/Verify tabs, PR-C Clients tab) and `Home.vue` (PR-B Get-started card, PR-D digest card). Merge PR-B first, then rebase PR-C and PR-D. +- **Phase 6 (PR-E)** depends on PR-B (starter endpoint), PR-C (`detected_unconnected_clients`) and PR-D (digest). +- Inside each phase, a `test` task precedes its `impl`. `[P]` tests in one group can be written in parallel. + +## Parallel examples + +- PR-A: T003, T005, T007, T009, T011, T012 and T013 are all test files in different packages, so they can be written together. Then come T004, T006, T008, T010, T014, ordered by import dependency: storage → onboarding → runtime → httpapi/telemetry. +- PR-D: T044, T046, T048 and T050 can be written in parallel (server, server, runtime, storage). T052 needs T045, T047, T049 and T051 merged into the branch. + +## Implementation strategy + +1. **MVP** = PR-A + PR-B (US5 + US1/US2 Web). It lifts the primary metric SC-001 and is measurable. +2. Add PR-C (US3) and PR-D (US4) in the same release if they are ready. +3. Add PR-E (native) in the same release or the next. +4. Read the E3 cohorts two releases after PR-B (T078).