docs(19.7-05): plan 05 summary; phase gate green, security review done, plugin test commits local

.planning/phases/19.7-generic-mcp-module/19.7-05-SUMMARY.md

@@ -0,0 +1,310 @@
+---
+phase: 19.7-generic-mcp-module
+plan: 05
+subsystem: testing
+status: complete
+tags: [pier, mcp, gate, security-review, bouncer, cabana, surf, wristband, sm-user-plugin, sm-oauthserver-plugin, sm-journal-plugin, sm-demo-app, vitest]
+
+requires:
+  - phase: 19.7-01
+    provides: dispatch marker, compiled handler lookup, stream route kind, account contributions, prompted record actions, wristband scope allow-list
+  - phase: 19.7-02
+    provides: modules/pier (catalog, dispatch, identity, sessions, fan-out, stateless path, PRM)
+  - phase: 19.7-03
+    provides: sm-user-plugin v0.1.10 (account link, frontend permission catalog, user MCP guard)
+  - phase: 19.7-04
+    provides: sm-oauthserver-plugin v0.1.2 (verifier, consent JSON), sm-journal-plugin v0.2.3 (MCP tools), demo wiring and TestMCPEndToEnd
+provides:
+  - scripts/check-phase19.7.sh, the fail-closed multi-repo phase gate (self-test, go, plugins, consumers, docker, admin, docs, marker), green in full on a Docker host
+  - modules/pier/helpers_test.go (fakeGuard, fakeVerifier, fakeLinkResolver, fakePersonalResolver, routePlugin, newApp, startInstance, twoInstances, sdkClient, fireSubjectChanged)
+  - named tests for every validation row in the framework, the three plugins and the demo
+  - 19.7-SECURITY.md (threats_open 0) and 19.7-SECURITY-REVIEW.md (T-19.7-01..41, T-19.7-SC)
+  - three framework fixes: cabana's backend middleware refuses a frontend marker, pier accepts tools/call without arguments, an ended pier session is never written back
+  - the filled 19.7-VALIDATION.md (nyquist_compliant true) and the two folded todos closed
+affects: [phase 19.7 verification (/msd-verify-work), framework v0.5.0 at phase completion, optional patch tags of the three plugins]
+
+actuals:
+  tokens: 62152
+  tasks: 4
+  commits: 9
+plan_head_before: 905111ba7240828f2434bf313439d5c8af0c78d1
+plan_head_after: 4730e37742223133033c3ff20a3cd0c527d77c53
+commit_ledger:
+  summercms.go: {before: 905111b, after: 4730e37, count: 9}
+  sm-user-plugin (sm-demo-app/plugins/golem15/user): {before: bfcfea4, after: 72de822, count: 1}
+  sm-oauthserver-plugin: {before: 8bbec72, after: 2929c91, count: 1}
+  sm-journal-plugin: {before: 4ab94da, after: a9a9b80, count: 1}
+  sm-demo-app: {before: c21ae9d, after: 6fa2d56, count: 1}
+
+tech-stack:
+  added: []
+  patterns:
+    - "Required-name gate entries as <package>:<TestName>, grouped per package so a package without a match can never pass as 'no tests to run'"
+    - "Postgres subtests are created only when not -short (no t.Skip), so the detector's skip refusal stays meaningful and the docker stage runs the same top-level name in full"
+    - "In-process dispatch marker as a test harness: account and user-guard routes are exercised without an admin or session row"
+    - "Session persistence rewrites, never creates (SET XX), so an ended session stays ended"
+    - "Pin check that tolerates a plugin submodule ahead of its pin only by *_test.go commits"
+
+key-files:
+  created:
+    - scripts/check-phase19.7.sh
+    - modules/pier/helpers_test.go
+    - modules/pier/tools_list_test.go
+    - modules/pier/catalog_test.go
+    - modules/pier/dispatch_test.go
+    - modules/pier/guard_rules_test.go
+    - modules/pier/sessions_test.go
+    - modules/pier/fanout_test.go
+    - modules/pier/endpoint_guards_test.go
+    - modules/bouncer/dispatch_test.go
+    - modules/cabana/dispatch_marker_test.go
+    - modules/cabana/account_contributions_test.go
+    - modules/cabana/record_action_prompt_test.go
+    - modules/cabana/scope_limit_for_test.go
+    - modules/surf/streaming_test.go
+    - modules/surf/pier_mount_test.go
+    - modules/wristband/mcp_scopes_test.go
+    - ../sm-demo-app/plugins/golem15/user/backend_link_test.go
+    - ../sm-demo-app/plugins/golem15/user/frontend_permission_catalog_test.go
+    - ../sm-demo-app/plugins/golem15/user/mcp_guard_test.go
+    - ../sm-oauthserver-plugin/mcp_verifier_test.go
+    - ../sm-oauthserver-plugin/mcp_consent_test.go
+    - ../sm-journal-plugin/mcp_test.go
+    - .planning/phases/19.7-generic-mcp-module/19.7-SECURITY.md
+    - .planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md
+  modified:
+    - modules/cabana/http.go
+    - modules/pier/dispatch.go
+    - modules/pier/session.go
+    - modules/pier/store.go
+    - modules/pier/store_memory.go
+    - modules/pier/store_redis.go
+    - docs/services/authentication.md
+    - modules/pier/tracer_smoke_test.go
+    - modules/pier/sessions_smoke_test.go
+    - modules/pier/identity_smoke_test.go
+    - modules/pier/export_test.go
+    - modules/cabana/administrators_harness_test.go
+    - modules/cabana/record_action_prompt_postgres_test.go
+    - modules/surf/streaming_smoke_test.go
+    - admin/tests/account/AccountContributions.test.ts
+    - admin/tests/form/RecordActionsPrompt.test.ts
+    - ../sm-demo-app/mcp_e2e_test.go
+    - .planning/phases/19.7-generic-mcp-module/19.7-VALIDATION.md
+    - .planning/phases/19.7-generic-mcp-module/deferred-items.md
+    - .planning/todos/done/2026-10-17-plugin-registered-frontend-permissions.md
+    - .planning/todos/done/2026-10-17-frontend-backend-account-link.md
+
+key-decisions:
+  - "P-21 implemented: gate stages self-test, go, plugins, consumers, docker, admin, docs, marker; docker last, NOT RUN only without a daemon, a failure with PHASE197_REQUIRE_DOCKER=1"
+  - "P-22 implemented: 19.7-SECURITY.md and 19.7-SECURITY-REVIEW.md in the 19.4 shape; threats_open 0"
+  - "P-23 verdict: superuser reach accepted as designed with audit and ceiling; mcp.refuse_superuser_backend not added, so no patch tag is needed for it (conditional follow-up in deferred-items)"
+  - "MCP grants without a consent row: accepted for this phase (AR-19.7-03); access ends through client revocation, user deletion or deactivation, expiry and the verifier; the e2e pins that the revoked event ends the session"
+  - "The user plugin's additive check uses both pre-phase bases (930b158 and v0.1.9); the user plugin runs with GOWORK=off, the OAuth server and blog plugins in their own workspaces"
+
+patterns-established:
+  - "A phase's last plan turns the plans' smokes into named tests the gate requires by package, untagged and with the build tag"
+
+requirements-completed: [SC-1, SC-2, SC-3, SC-4, D-05, D-08, D-17, D-20, D-21, D-22, D-25, D-26, D-27, D-31, D-36]
+
+coverage:
+  - id: D1
+    description: "The fail-closed multi-repo gate passes in full on a Docker host and its detectors are proven on planted output"
+    requirement: "D-31"
+    verification:
+      - kind: other
+        ref: "bash scripts/check-phase19.7.sh --all (PASS self-test, go, plugins, consumers, admin, docs, marker, docker)"
+        status: pass
+    human_judgment: false
+  - id: D2
+    description: "Every framework validation row has its named test (pier, bouncer, cabana, surf, wristband, vitest)"
+    requirement: "SC-3"
+    verification:
+      - kind: unit
+        ref: "modules/pier/{tools_list,catalog,dispatch,guard_rules,sessions,fanout,endpoint_guards}_test.go"
+        status: pass
+      - kind: unit
+        ref: "modules/bouncer/dispatch_test.go#TestDispatchMarker, modules/cabana/dispatch_marker_test.go#TestDispatchMarker"
+        status: pass
+      - kind: unit
+        ref: "modules/surf/{streaming,pier_mount}_test.go, modules/wristband/mcp_scopes_test.go, modules/cabana/{account_contributions,record_action_prompt,scope_limit_for}_test.go"
+        status: pass
+    human_judgment: false
+  - id: D3
+    description: "Plugin and demo rows: link proof, frontend permission catalog, user guard, verifier, consent, Journal declaration, end-to-end revoke matrix"
+    requirement: "SC-4"
+    verification:
+      - kind: integration
+        ref: "sm-user-plugin TestBackendLink, TestFrontendPermissionCatalog, TestUserMCPGuard (Postgres)"
+        status: pass
+      - kind: integration
+        ref: "sm-oauthserver-plugin TestMCPTokenVerifier, TestMCPConsent (Postgres)"
+        status: pass
+      - kind: unit
+        ref: "sm-journal-plugin TestJournalMCP"
+        status: pass
+      - kind: e2e
+        ref: "sm-demo-app TestMCPEndToEnd (tagged, Postgres)"
+        status: pass
+    human_judgment: false
+  - id: D4
+    description: "Security review of T-19.7-01..41 and T-19.7-SC with evidence; superuser verdict recorded"
+    requirement: "D-22"
+    verification:
+      - kind: other
+        ref: ".planning/phases/19.7-generic-mcp-module/19.7-SECURITY-REVIEW.md, 19.7-SECURITY.md (threats_open 0)"
+        status: pass
+    human_judgment: false
+
+duration: about 95 min
+completed: 2026-10-23
+---
+
+# Phase 19.7 Plan 05: Unit tests, phase gate and security review Summary
+
+**A fail-closed gate across six repositories (`scripts/check-phase19.7.sh`) now requires a named test for every row of the validation map, and it passes in full on a Docker host. The security review covers every threat of the phase. The tests and the review found three framework defects, each fixed in its own commit: cabana's `backend` middleware accepted a frontend dispatch marker, a pier session that had ended could be written back by a request still in flight, and pier rejected argument-less tool calls.**
+
+## Performance
+
+- **Duration:** about 95 minutes (2026-10-23, roughly 11:05Z to 12:40Z)
+- **Tasks:** 4 of 4
+- **Repositories touched:** 5 with commits (summercms.go, sm-user-plugin, sm-oauthserver-plugin, sm-journal-plugin, sm-demo-app)
+
+## Accomplishments
+
+- **Task 1 (tracer):** `modules/pier/helpers_test.go` collects the plan 02 fixtures and adds configurable fakes and the two-instance miniredis harness. `TestToolsList` covers per-principal lists, the read ceiling, a tool with no subject, unlisted calls that never reach the route, and the union of tools when an identity has both a frontend and a backend subject. `TestDispatchMarker` exists in both bouncer and cabana; the cabana one has a Postgres part. The gate shipped at this point with its self-test, go and marker stages.
+- **Task 2:** `TestCatalog`, `TestDispatch`, `TestGuardRules`, `TestSessions` (on Redis across two instances and on the memory store), `TestFanOut`, `TestStreamingRoute`, `TestPierMount` (both builds), `TestMCPScopes`, `TestAccountContributions` (with a Postgres transaction case and a check that the existing payloads are unchanged), `TestRecordActionPrompt` and `TestScopeLimitForMethods`, plus extended vitest files. The gate's admin stage went live.
+- **Task 3:** `TestBackendLink`, `TestFrontendPermissionCatalog` and `TestUserMCPGuard` in the user plugin; `TestMCPTokenVerifier` and `TestMCPConsent` in the OAuth server plugin; `TestJournalMCP`, which compiles Journal's declaration through pier itself; and a revoke matrix in the demo's `TestMCPEndToEnd`. The gate gained its plugins (with the additive-export checks), consumers, docker and docs stages. The validation map is filled in (`nyquist_compliant: true`).
+- **Task 4:** wrote `19.7-SECURITY-REVIEW.md` and `19.7-SECURITY.md` (`threats_open: 0`). The review added pins for the endpoint's outer layers in `TestEndpointGuards` and a check that undeclared argument keys are refused, and recorded the superuser verdict. The two folded todos moved to `todos/done/` with closing notes.
+
+## Task Commits
+
+summercms.go (master, not pushed):
+
+1. **Task 1:** `c49983d` fix(cabana): frontend dispatch marker refused by the backend middleware; `3bbbe6c` test: gate (go, marker stages), pier helpers, TestToolsList, TestDispatchMarker
+2. **Task 2:** `c29d228` fix(pier): tools/call without arguments; `1111828` fix(pier): ended session never written back; `f14eae9` test: framework tests for every validation row, admin stage
+3. **Task 3:** `a94bdc1` test: gate stages plugins, consumers, docker, docs; `d621ef1` docs: validation map
+4. **Task 4:** `0b8e027` test: security review pins; `4730e37` docs: security review and contract, deferred items, todos closed
+
+Plugins and demo (local commits, **not pushed and not tagged**):
+
+| Repo | Commit | Content |
+|------|--------|---------|
+| sm-user-plugin (checkout at `sm-demo-app/plugins/golem15/user`) | `72de822` | TestBackendLink, TestFrontendPermissionCatalog, TestUserMCPGuard |
+| sm-oauthserver-plugin | `2929c91` | TestMCPTokenVerifier, TestMCPConsent |
+| sm-journal-plugin | `a9a9b80` | TestJournalMCP |
+| sm-demo-app | `6fa2d56` | TestMCPEndToEnd revoke matrix |
+
+All four add tests only; none changes shipped code. Patch tags (sm-user-plugin v0.1.11, sm-oauthserver-plugin v0.1.3, sm-journal-plugin v0.2.4) are optional and wait for the user. The demo's `plugins/golem15/user` working tree sits one commit (72de822) ahead of its v0.1.10 pin. I did not commit that pointer change in sm-demo-app. The gate's pin check reports the drift and accepts it only because the commit touches `*_test.go` files alone.
+
+`commits: 9` is measured in summercms.go (`git rev-list --count 905111b..4730e37`); the four plugin and demo commits come on top of that count.
+
+## Gate output (`bash scripts/check-phase19.7.sh --all`, final run after Task 4)
+
+```
+PASS self-test
+PASS go
+PASS plugins
+pins: plugins/golem15/user is ahead of its pin bfcfea4ed7d4 by test-only commits (release not needed for shipped code)
+check-deploy: ok
+PASS consumers
+PASS admin
+PASS docs
+PASS marker
+PASS docker
+Phase 19.7 gate passed
+```
+
+Exit 0. These also pass: `bash scripts/check-phase19.6.1.sh --all` (every stage PASS, docker included), and in the framework `go vet ./...` plus the full `go test -count=1 ./...` with Docker.
+
+## Deviations from Plan
+
+### Auto-fixed Issues
+
+**1. [Rule 1 - Bug, security] cabana's `backend` middleware honoured a frontend dispatch marker**
+- **Found during:** Task 1 (cabana `TestDispatchMarker`, in-process admin API case)
+- **Issue:** `bouncer.Registry.Middleware("backend")` honoured any marker that listed `backend`, even one whose principal had `Backend: false`. The admin API would then have treated a frontend user id as an administrator id. `ResolveBackend` already refused such a marker; the middleware did not.
+- **Fix:** `refuseFrontendMarker` wraps the backend guard middleware and answers 401 for that case. The authentication docs page now states the rule.
+- **Files modified:** modules/cabana/http.go, docs/services/authentication.md
+- **Commit:** c49983d
+
+**2. [Rule 1 - Bug] `tools/call` without arguments was refused**
+- **Found during:** Task 2 (`TestDispatch`)
+- **Issue:** a JSON `null` for arguments was validated against the object schema and refused, so an agent could not call a tool that has no required input unless it sent `{}`.
+- **Fix:** an empty body or `null` now means `{}`.
+- **Files modified:** modules/pier/dispatch.go
+- **Commit:** c29d228
+
+**3. [Rule 1 - Bug, security] an ended session could be written back**
+- **Found during:** Task 2 (`TestFanOut`, failed about one run in a hundred)
+- **Issue:** after a request finished, its session state was saved with a plain Put. If the session had ended while the request ran (DELETE, a revoked or deleted event, expiry), the save recreated the record and the next request rehydrated the ended session.
+- **Fix:** an unexported `updater` (Redis `SET XX`, memory update-if-present). Persisting now only rewrites an existing record and closes the local copy when the record is gone. TestFanOut, TestSessions and both session smokes passed 300 times in a row afterwards.
+- **Files modified:** modules/pier/session.go, store.go, store_memory.go, store_redis.go
+- **Commit:** 1111828
+
+**4. [Rule 3 - Blocking] pier helpers are in package `pier_test`, not `pier`**
+- The plan named `package pier`, but the plan 02 smokes the helpers serve are in `pier_test`, which imports surf. Internal seams stay in `export_test.go` (package pier): `AdvanceStoreClock`, `StatelessServers`, `SetRecheck`.
+
+**5. [Rule 3 - Blocking] cabana's `TestDispatchMarker` lives in `cabana_test`**
+- The Postgres harness (`newScreensEnv`) is in the external package. The harness now keeps its `*backpack.App` (a one-field test change). For the same reason, `TestAccountContributions` is also in `cabana_test`.
+
+**6. [Rule 3] Postgres subtests are created only when not `-short`**
+- The gate's detector refuses skipped subtests. So `TestDispatchMarker`, `TestAccountContributions` and the other tests with a Postgres part return before creating that part under `-short`, and the docker stage runs the same top-level name in full.
+
+**7. [Rule 2] `TestScopeLimitForMethods` (cabana) stands in for the backend guard's scoped principal case**
+- The built-in backend provider needs a database (`cabana.BackendPrincipal`), so it cannot run under `-short`. `TestGuardRules` proves that pier asks a method-aware provider with each tool's method. `TestScopeLimitForMethods` proves that cabana's per-method filter is the one the backend guard uses.
+
+**8. [Rule 3] `TestRecordActionPrompt`'s case for the `RecordAction` wrapper is pinned through the Postgres route test**
+- The wrapper needs a database. `TestRecordActionPromptRun` gained a plain action that runs with an empty Value.
+
+**9. [Rule 1 - test] the streaming smoke reset the mount hook to nil**
+- In a build with the tag, that broke every later surf test. The smoke now restores the previous hook. This is a test-only change, committed in f14eae9.
+
+**10. [Rule 3] the gate's consumers stage accepts a submodule that is ahead of its pin only by test commits**
+- The demo's `check-pins.sh` correctly reports the user plugin checkout at 72de822 against the v0.1.10 pin. The gate accepts the drift only when the submodule descends from its pin and the diff touches nothing but `*_test.go` files; any other mismatch fails.
+
+**11. [Rule 2] the gate also checks `account.go` and runs the demo's deploy check**
+- The go stage refuses any change to `modules/cabana/account.go` since 51d088a (D-04: payloads unchanged). The consumers stage runs `check-deploy.sh` (T-19.7-37).
+
+**Total deviations:** 3 production fixes (two security-relevant), 8 adjustments to test structure or the gate. No architectural change.
+
+## Issues Encountered
+
+- `cp` in this shell is interactive (`cp -i`); one backgrounded debug loop hung on its prompt and was killed. No repository content was affected.
+- With `GOWORK=off` the user plugin and the demo's release build compile against summercms v0.4.4, which does not include this plan's framework fixes. The gate runs the demo through its workspace (the framework checkout); see deferred items.
+
+## Known Stubs
+
+None.
+
+## Threat Flags
+
+None beyond the phase threat model. This plan adds tests, the gate and three fixes that narrow existing surfaces.
+
+## Deferred Items
+
+Recorded in `deferred-items.md`:
+- framework fixes c49983d, c29d228 and 1111828 are on master only; applications pinning v0.4.4 lack them until the next framework tag (v0.5.0 at phase completion)
+- the four plugin and demo test commits are local and untagged (optional patch tags)
+- a superuser refusal switch, conditional on a future superuser-only or destructive backend tool
+- the decision on MCP grants that have no consent row (AR-19.7-03; the follow-up stays open)
+
+## g15office (for the orchestrator's phase-completion step)
+
+The two closed todos correspond to g15office tasks **`hmguritm`** (plugin-registered frontend permissions) and **`tqyvnzxx`** (frontend-backend account link). Both move into the phase milestone task **`dcnv4mha`** with a one-line comment. The phase-advance comment also belongs to that step (`.planning/G15OFFICE.md`). The executor did not touch g15office.
+
+## User Setup Required
+
+None. Push, tags and the framework v0.5.0 release wait for the user's go-ahead.
+
+## Next Phase Readiness
+
+`/msd-verify-work` can run the gate: `bash scripts/check-phase19.7.sh --all` on a Docker host. After that, phase completion: framework v0.5.0 (which carries plans 01-05 and this plan's fixes), then the demo bump and deploy.
+
+---
+*Phase: 19.7-generic-mcp-module*
+*Completed: 2026-10-23*
+
+## Self-Check: PASSED
+
+All 25 listed created files exist on disk; the framework commits c49983d, 3bbbe6c, c29d228, 1111828, f14eae9, a94bdc1, d621ef1, 0b8e027 and 4730e37 and the plugin and demo commits 72de822, 2929c91, a9a9b80 and 6fa2d56 exist in their repositories; measured framework commits 905111b..4730e37 = 9.