From 33c40ff483911bcfd824d66971cae478b2245279 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 23 Oct 2026 21:29:07 +0200 Subject: [PATCH] docs(19.7): add re-review fix report --- .../19.7-REREVIEW-FIX.md | 377 ++++++++++++++++++ 1 file changed, 377 insertions(+) create mode 100644 .planning/phases/19.7-generic-mcp-module/19.7-REREVIEW-FIX.md diff --git a/.planning/phases/19.7-generic-mcp-module/19.7-REREVIEW-FIX.md b/.planning/phases/19.7-generic-mcp-module/19.7-REREVIEW-FIX.md new file mode 100644 index 00000000..a1f59d51 --- /dev/null +++ b/.planning/phases/19.7-generic-mcp-module/19.7-REREVIEW-FIX.md @@ -0,0 +1,377 @@ +--- +phase: 19.7-generic-mcp-module +review: 19.7-REREVIEW.md +fixed: 2026-10-23 +iteration: 1 +findings_in_scope: 28 +status: all_fixed_or_deferred +summary: "1 critical and 10 warnings fixed, each with a test that fails on the old code; info items fixed where cheap, the rest deferred with a reason in deferred-items.md" +--- + +# Phase 19.7 re-review fix report + +The user chose the code-review fixer route (2026-10-23) with cheap info findings included. Three fixer lanes ran in sequence on the main checkouts: A (framework MCP core), B (cabana and admin SPA), C (plugins and demo, including the framework seam for RC-WR-01). Every deferral is also recorded in `deferred-items.md`. + +Final gate (`bash scripts/check-phase19.7.sh --all`, after lane C): every stage PASS, verdict "Phase 19.7 gate passed WITH UNRELEASED PINS: the release pins are not yet proven (plugins/golem15/journal plugins/golem15/oauthserver plugins/golem15/user)". The qualified verdict is expected until the release. + +Items flagged for human verification at UAT: +- RB-WR-01: the save shortcuts inside a plugin field type's shadow root are verified in happy-dom only; check in Firefox. +- RC-CR-01 and RC-WR-01: the password-change revocation and the consented-administrator binding change revocation logic and add per-user advisory locks; worth a human look. +- RC-WR-01 behaviour change: backend grants issued before oauthserver migration 202610230001 lose backend reach until the agent reconnects. + + +# Lane A + + +## Phase 19.7: Code Review Fix Report (re-review, lane A) + +**Fixed at:** 2026-10-23 +**Source review:** .planning/phases/19.7-generic-mcp-module/19.7-REREVIEW.md (section "# Lane A") +**Iteration:** 1 + +**Summary:** +- Findings in scope: 12 (6 warnings, 6 info) +- Fixed: 12. RA-IN-04 is only partly fixed; its leftover part is deferred. +- Skipped: 0 +- Deferred residuals (recorded in deferred-items.md, commit eea04b1): the check-then-act cap in RA-IN-04, and the optional upstream check in RA-WR-06 + +**Where verification ran:** in the main checkout. `workflow.use_worktrees` is false, so no worktree was created. Every commit was checked with `go vet ./...`, `go vet -tags summercms_mcp ./...` and `go test -short ./...`, all green. `go test ./modules/pier/...` (with `-race` for the concurrency changes) and `go test ./cmd/summer -run TestDocsTree` were run after each pier or doc change. `scripts/check-phase19.7.sh --self-test` passes. I also ran `--demo-pins` and `--consumers` against the real sibling checkouts. They exit 0 and print the qualified line. + +Each warning fix has a test that I first ran against the reverted code. Every one failed there, and passes with the fix. + +## Fixed Issues + +### RA-WR-01: Event fan-out nests session locks under the router lock; guard I/O under those locks has no deadline + +**Files modified:** `modules/pier/events.go`, `modules/pier/session.go`, `modules/pier/stateless.go`, `modules/pier/config.go`, `modules/pier/README.md`, `docs/services/mcp.md`, `modules/pier/lock_scope_test.go` +**Commit:** cccf402 +**Applied fix:** +- **Matching.** `matchingFunc` on both paths now copies the entries while it holds `sr.mu`/`sp.mu`, releases the lock, and then reads each identity. The identity sits in a new `identityBox`, which has its own mutex and is never held across I/O. Matching never takes `ls.mu`/`e.mu`. +- **Deadlines.** `widen`, `recomputeSession` and `recomputeStateless` run their guard checks under `guardContext`, which is `context.WithTimeout(context.WithoutCancel(ctx), cfg.CallTimeout)`. The README and the docs now say that `mcp.call_timeout` also bounds these re-filter checks. + +**Test:** `TestLockScopeUnderWidening` +- "an event for the blocked session does not stall another session": failed before the fix with "another session's call waited for the blocked widening". +- "a blocked widening ends at mcp.call_timeout": failed before the fix, because the guard check outlived `call_timeout`. + +### RA-WR-02: The fingerprint includes the OAuth token id, so a shared stateless server flips its tool set + +**Files modified:** `modules/pier/identity.go`, `modules/pier/stateless.go`, `modules/pier/events.go`, `modules/pier/README.md`, `modules/pier/widening_test.go`, `modules/pier/fingerprint_test.go` (new) +**Commit:** f604e6d +**Applied fix:** +- **Fingerprint.** It no longer includes subject token ids, so an access-token rotation alone re-filters nothing. +- **Stateless refresh.** It now only adds tools and prompts. It runs when a fingerprint is new to the entry, and the entry remembers it in `widened`. A fan-out recompute clears that list and can still narrow. The receiving middleware still filters every list and call. +- **Stateful path.** It keeps its exact add-and-remove `widen`, because a session serves one client. +- **LRU key.** I did not re-key the LRU by binding plus fingerprint. The add-only refresh fixes the race without it. + +**Tests:** +- `TestFingerprintIgnoresTokenID` (internal). +- `TestStatelessSharedBinding`: a wide token's call is blocked after its server was chosen, then a narrow token sends a request. Before the fix the call failed with `unknown tool "acme_echo"`. + +### RA-WR-03: A transient guard error during widening permanently narrows a live session + +**Files modified:** `modules/pier/filter.go`, `modules/pier/events.go`, `modules/pier/stateless.go`, `modules/pier/README.md`, `modules/pier/widening_test.go` +**Commit:** e7630e6 +**Applied fix:** +- **New check.** `catalog.allowedReport` returns the allowed lists plus the first provider error. `allowed` is now built on it. +- **Widening, both paths.** On an error it logs "pier: widening deferred" and leaves the server and its recorded fingerprint untouched, so the next request retries. +- **Fan-out recompute.** It still applies its diff, which fails closed and is safe for an event. On an error it records no fingerprint, so the binding's next request re-filters. + +**Test:** `TestWideningGuardError`, with subtests "widening on stateless", "widening on stateful" and "recompute on an event". All three failed before the fix and listed `[acme_show]` or `[]` after the provider recovered. + +### RA-WR-04: LRU capacity eviction ends live listen streams and in-flight calls + +**Files modified:** `modules/pier/stateless.go`, `modules/pier/README.md`, `modules/pier/stateless_fanout_test.go` +**Commit:** 928731d +**Applied fix:** +- **Counting active requests.** Each stateless request carries an `entryUse` on its context. `getServer`/`hit` count the entry they hand out as active while holding `sp.mu`. `ServeHTTP` releases it when the request ends, so a listen stream keeps its entry active for its whole life. +- **Eviction.** It walks the LRU from the back and drops only idle entries. If every entry is busy, the LRU stays above capacity until requests end (documented). +- **Revocation.** `evictFunc` is unchanged, so a revocation still closes every stream. + +**Test:** `TestStatelessFanOut/an LRU eviction spares a server with a listen stream`. This rewrites the subtest that pinned the old behaviour. When the binding past capacity arrives, the listener's server stays and the oldest idle one is evicted. A revocation then still ends the stream. Before the fix: "the LRU eviction ended a live listen stream". + +### RA-WR-05: Prompt Render errors reach the MCP client verbatim + +**Files modified:** `modules/pier/session.go`, `modules/pier/README.md`, `docs/services/mcp.md`, `modules/pier/prompt_ceiling_test.go` +**Commit:** 788d4a4 +**Applied fix:** `promptHandler` logs the error with the prompt name ("pier: prompt render failed"). It then returns `&jsonrpc.Error{Code: CodeInternalError, Message: "pier: the prompt could not be rendered"}`. +**Test:** `TestPromptRenderError` (stateful and stateless). Before the fix the client received `failed to connect to host=db.internal ...`. + +### RA-WR-06: The gate passes while demo submodules carry unreleased non-test commits + +**Files modified:** `scripts/check-phase19.7.sh` +**Commit:** 2fdda99 +**Applied fix:** +- **Recording.** `demo_pins` records each submodule it accepts as ahead of its pin by unreleased commits. It keeps them in `UNRELEASED_PINS` and, when `PHASE197_PINS_FILE` is set, also writes them to that file, which `run_all` exports. +- **`--consumers`.** With unreleased pins it now ends with "phase19.7 consumers passed WITH UNRELEASED PINS: (the release pins are not yet proven)". +- **`--all`.** It ends through `gate_verdict`. With unreleased pins that prints a notice box and the last line "Phase 19.7 gate passed WITH UNRELEASED PINS: the release pins are not yet proven ()". With none it prints the unqualified "Phase 19.7 gate passed". +- **Header comment.** Updated to describe this. +- **Exit status.** It stays 0, and fail-closed by default (`PHASE197_ALLOW_UNRELEASED`) was not added. The release order verifies the phase before anything is tagged. A non-zero exit would fail every pre-release run, and the user's rule only asked for a qualified verdict. A check that greps for the exact line "Phase 19.7 gate passed" now fails while pins are unreleased. + +**Test:** new self-test cases: +- `pins-test-only-unrecorded` +- `pins-unreleased-recorded` +- `verdict (unqualified only without unreleased pins)` + +Against the real checkouts, journal, oauthserver and user are reported, and the consumers stage prints the qualified line. + +### RA-IN-01: lazyStore's doc comment says nothing dials at mount + +**Files modified:** `modules/pier/store_redis.go`, `modules/pier/pier.go` +**Commit:** 38e028f +**Applied fix:** +- The `lazyStore` and `newStore` comments now describe the mount-time background warm-up and its logged retries. +- Decision: keep the warm-up for commands that mount pier without serving, such as `route:list`. It only dials and logs until the process exits. This decision is recorded in the comment. +- The README row that said the store opens "on the first request" is corrected in 58408aa. + +### RA-IN-02: Stream re-checks resolve the bearer without the client address + +**Files modified:** `modules/pier/session.go`, `modules/pier/stateless.go`, `modules/pier/README.md`, `modules/pier/stateless_fanout_test.go` +**Commit:** afc7510 +**Applied fix:** The GET stream and the listen stream capture `p.clientIP(r)` when they open. They resolve every re-check with `towel.WithClientIP`. +**Test:** `TestStreamRecheckClientIP`, which failed before the fix: the re-check verifications saw an empty address. + +### RA-IN-03: reserveScript touches keys it does not declare + +**Files modified:** `modules/pier/store_redis.go`, `modules/pier/README.md`, `docs/services/mcp.md` +**Commit:** 58408aa +**Applied fix:** I took the documentation option. The store needs a standalone Redis. Redis Cluster is not supported, and a key-pattern ACL must allow the whole `mcp:` range. This is stated in the script comment, the README `mcp.sessions.store` row and the docs config table. + +### RA-IN-04: The non-reserver fallback is check-then-act and briefly exposes an unrecorded session + +**Files modified:** `modules/pier/session.go` +**Commit:** 034aa19 +**Applied fix:** This is partial. The fallback now calls `Put` right after the Count check and before the server is built. A failed connect or initialize deletes the record again. The session is never in `sr.local` without its record, so a reaper pass can no longer close it as expired. The existing `TestSessionReserve` fallback subtest passes. +**Deferred residual:** the Count-then-Put cap is still racy. Closing it needs `Reserve` in the exported `pier.Store` interface, or a refusal at mount, which changes pier's exported API for a branch only a test wrapper reaches. Recorded in deferred-items.md (eea04b1). + +### RA-IN-05: A backend token whose scopes no longer resolve gets "account unknown or deactivated" + +**Files modified:** `modules/pier/backend_guard.go`, `modules/pier/dispatch.go`, `modules/pier/README.md` +**Commit:** c0ce07e +**Applied fix:** +- **Sentinel.** The backend guard's `principal` now answers `errScopeUnavailable` when the token's stored scopes resolve to nothing. Before, it answered a nil principal. +- **Dispatch.** It maps this to the fixed tool error "pier: the token's scopes no longer grant this route". +- **Listing.** `MCPAllowsMethod` still treats it as a refusal, so the RA-WR-03 retry logic does not see it as a failed check. + +No unit test: the path needs cabana's database-backed `BackendPrincipal`. + +### RA-IN-06: demo_pins passes when check-pins.sh exits non-zero with no output + +**Files modified:** `scripts/check-phase19.7.sh` +**Commit:** 99f5ed4 +**Applied fix:** `demo_pins` counts the mismatch lines it accepts. It refuses when `check-pins.sh` fails without one. +**Test:** new self-test `pins-silent-failure`, which plants a silent `exit 1` `check-pins.sh`. + +## Deferred + +- **RA-IN-04 residual:** the check-then-act cap of the non-reserving fallback. Fixing it is an exported `pier.Store` API change for a branch only tests reach. +- **RA-WR-06 optional `git branch -r --contains` upstream check:** the gap-closure commits are deliberately unpushed until the release order runs, so this check would refuse every pre-release run. `PHASE197_RELEASED=1` after the tags is the release proof. + +Both are appended to `.planning/phases/19.7-generic-mcp-module/deferred-items.md` in the docs commit eea04b1. + +--- + +_Fixed: 2026-10-23_ +_Fixer: Claude (msd-code-fixer)_ +_Iteration: 1_ + +# Lane B + + +## Phase 19.7: Code Review Fix Report (re-review, Lane B) + +**Fixed at:** 2026-10-23T18:39:41Z +**Source review:** .planning/phases/19.7-generic-mcp-module/19.7-REREVIEW.md (section "# Lane B" only) +**Iteration:** 1 + +**Summary:** +- Findings in scope: 6 (RB-WR-01, RB-WR-02, RB-IN-01..04) +- Fixed: 5 (RB-IN-03 counted as skipped: the code fix stays deferred; only its documentation half landed) +- Skipped/deferred: 1 (RB-IN-03 code fix) + +**Where verification ran:** in the main checkout (`workflow.use_worktrees: false`, so no worktree was created). Before every commit: `go vet ./...` and `go test -short ./...`. For admin changes also `npm --prefix admin run test` (101 files) and `npm --prefix admin run typecheck`. `scripts/check-admin-openapi.sh` regenerated the OpenAPI files for RB-IN-02, and `--check` exits 0. `go test ./cmd/summer -run TestDocsTree` passed after each doc change. `scripts/check-admin-dist.sh` matches after the dist commit 34dce1c. + +## Fixed Issues + +### RB-WR-01: The draft-commit blur does not reach an input inside a plugin field type's shadow root + +**Files modified:** `admin/src/app/hotkeys.ts`, `admin/tests/app/hotkeysEditing.test.ts` +**Commit:** 3afe9fa (dist: 34dce1c) +**Applied fix:** Before clicking, the save shortcut now focuses the target button with `preventScroll`, as a pointer press does. This unfocuses an inner input in an open or a closed shadow root, so its blur and change handlers run. If the target takes no focus, the shortcut calls `blur()` on the deepest focused element instead: a private `deepestActiveElement` helper follows `shadowRoot.activeElement` through open roots. After the click, focus goes back to that element, unless the click moved focus somewhere other than the target or body. +**Test:** `save shortcuts commit a draft inside a shadow root` (4 cases: Ctrl+S and Ctrl+Enter, open and closed root). The test field emits `summer-change` from the inner input's `change` event. All 4 cases fail on the previous code. happy-dom behaves like the spec and Firefox here: `blur()` on the host is a no-op. +**Status:** fixed: requires human verification (focus and blur behaviour; it has only been checked in happy-dom, not in Firefox) + +### RB-WR-02: A scope name is validated trimmed but registered untrimmed + +**Files modified:** `modules/cabana/token_scopes.go`, `modules/cabana/token_scope_name_test.go` +**Commit:** a5ce43f +**Applied fix:** `validatePluginTokenScope` now checks `scope.Name` exactly as `compileTokenScopes` registers it, with `pact.ValidTokenScopeName`. A padded name now fails boot instead of being trimmed. This matches the pact rule, and the docs claim that "Both catalogs check names with `pact.ValidTokenScopeName`" now holds. The duplicate check uses the same key that is registered. No doc change was needed. +**Test:** `TestTokenScopeNameParity` now also covers `" read"`, `"read "`, `"read\n"` and `"\tblog:read"`. On the old code it fails for all four. + +### RB-IN-01: The SPA URL check passes `//host` + +**Files modified:** `admin/src/components/account/AccountContributions.vue`, `admin/tests/account/AccountContributions.test.ts` +**Commit:** 633d450 (dist: 34dce1c) +**Applied fix:** `safeUrl` now refuses any ASCII control character (U+0000-U+001F, U+007F) first, as Go's `url.Parse` does on the server. An absolute http(s) URL must also parse with a host. With both changes the SPA and the server apply the same rule, as the comment says. +**Test:** `a "" URL answer renders no link and never opens a window`, for `/\t/evil.example/x`, `/\n/evil.example`, `/\r\n/evil.example`, `/x\u0000` and `https://`. All fail on the old code. + +### RB-IN-02: An action whose Applies fails disappears with no signal + +**Files modified:** `modules/cabana/account_contributions.go`, `modules/cabana/account_contributions_hardening_test.go`, `modules/cabana/admin_openapi.go`, `modules/cabana/README.md`, `docs/backend/users-and-permissions.md`, `admin/openapi/admin.json`, `admin/src/api/schema.d.ts` +**Commit:** 81342f1 +**Applied fix:** When an action's `Applies` fails, the action is still dropped and `view.Error = true` is now set on its section. The SPA already shows the section error line next to a successful status. The field description of `AccountContributionView.Error`, the cabana README (feature entry and route table) and the My account docs are updated. The OpenAPI document and the schema types are regenerated; this was a description change only. +**Test:** `TestAccountContributionsAppliesFailureFlagsSection` fails on the old code. `TestAccountContributionsIsolation` still passes. + +### RB-IN-04: The custom-element-in-form rule also mutes Esc on focusable non-field widgets + +**Files modified:** `admin/src/app/hotkeys.ts` +**Commit:** 89b85f5 (dist: 34dce1c) +**Applied fix:** The trade-off is now documented in the header comment and in the `isEditableTarget` doc comment. The optional narrowing to plugin field types was not done. With it, Esc would leave the form while the admin types into a `type: widget` element with an input in a closed root. This is recorded in deferred-items.md. + +## Skipped Issues + +### RB-IN-03: The CLI `admin:token:revoke` fires no revoke event + +**File:** `modules/cabana/token_commands.go:225-255` +**Reason:** The code fix stays deferred because it is not cheap. Calling `fireTokenRevoked` from the command would do nothing. The command runs in its own process, which does not mount pier, so no listener receives the event and nothing is published to the store a running server reads. Closing the gap needs the command to publish into pier's store directly, which is a seam across modules. The reviewer's alternative, documenting the gap, was applied in 4d7eb9d: the cabana README events entry and the command table now say that only the My account revoke fires the event, and that the CLI-revoked token's sessions end at its next verification. The existing deferred-items entry was updated in 0b0cae5. +**Original issue:** The operator revoke command fires nothing, so the token's open sessions stay until the next bearer re-verification. The README did not say this applies to the HTTP route only. + +--- + +_Fixed: 2026-10-23T18:39:41Z_ +_Fixer: Claude (msd-code-fixer)_ +_Iteration: 1_ + +# Lane C + + +## Phase 19.7: Code Review Fix Report (re-review, lane C) + +**Fixed at:** 2026-10-23 +**Source review:** 19.7-REREVIEW.md, section "# Lane C" only (RC-CR-01, RC-WR-01..02, RC-IN-01..07) +**Iteration:** 1 + +**Summary:** +- Findings in scope: 10 +- Fixed (fully or in their cheap part): 8 (RC-CR-01, RC-WR-01, RC-WR-02, RC-IN-01, RC-IN-02, RC-IN-03, RC-IN-04, RC-IN-05), plus the cheap part of RC-IN-06 +- Deferred: RC-IN-06 (boot-order part), RC-IN-07 (release-time); also the optional parts of RC-CR-01 and RC-IN-01 + +Repositories: U = sm-user-plugin (sm-demo-app/plugins/golem15/user), O = sm-oauthserver-plugin, J = sm-journal-plugin, V = vue-demo-app, D = sm-demo-app, F = summercms.go. + +Verification ran in the main checkouts (`workflow.use_worktrees: false`, no worktree created). Each repo's `go vet ./...` and `go test -short ./...` were green before every commit, and the Docker-backed tests of every changed package ran against Postgres (testcontainers). For each fix the new test was also run against the old code (stash, or a temporary mutation that was then restored) and failed there. + +## Fixed Issues + +### RC-CR-01: Self-service password change kept OAuth/MCP grants refreshed after the session's iat + +**Status:** fixed: requires human verification (logic) +**Commits:** U 30f7a01 (fix + README + test), O 5624e36 (test), O 95ccaec (README test line) +**Applied fix:** `ChangePassword` fires `AccessRevokedEvent` with `Before = time.Now()` (the request instant). `tokens_valid_after` is still `iat - 1s`, so the calling session survives. The README now says that `Before` is the revocation instant for every reason, and that the self-service cutoff only keeps the calling session. +**Tests:** +- U `TestAccessRevocation` / "ChangePassword revokes as of the request and keeps the calling session". The session is crafted with an iat 20 minutes old; the test asserts Before >= request start, the stored cutoff equals iat-1s, and `/fetch` still works with the session. On the old code it fails ("Before ... is not the instant of the request"). +- O `TestMCPGrantRevocation` / "a password change ends a grant rotated after the session's iat". It goes through `POST /change-password` after the agent rotated its grant, then asserts all rows are revoked, the verifier refuses the rotated token, the rotated refresh is refused and the session works. On the old code it fails with "1 revoked, 3 live". + +### RC-WR-01: backend grants could act as an administrator the consent did not show + +**Status:** fixed: requires human verification (logic, concurrency) +**Commits:** +- F c37c6ce `fix(19.7): RC-WR-01 ...`. Adds `pact.MCPGrant.BackendUserID`. pier's `resolveOAuth` and `refreshBackend` (after a link event) create a backend subject only when the current link names that id. Zero means no backend subject (fail closed). The pact README, pier README and docs/services/mcp.md were updated in the same commit, and the docs checker passes. +- O 105d263: migration `202610230001_add_golem15_oauthserver_backend_user_id` adds a nullable `backend_user_id` to the mcp_codes, access_tokens and refresh_tokens tables (no backfill: pre-migration backend grants stop acting as an admin until reconnected, as the README upgrade note says). Consent decide takes `backend_admin` (the shown login) and answers 409 `backend_link_changed` when it is not the admin linked now (moved, removed, or not named while linked). The linked admin id goes through `classes.WithConsentedBackend(ctx)` into `MarkIssued`. The code exchange and every rotation copy it onto the tokens, and the verifier returns it. `OnBackendLinkChanged` also deletes unexchanged backend MCP codes. `revokeUserGrants` takes per-user advisory locks (exclusive, ascending id); the code exchange and the refresh rotation take the same lock shared before their row lock, so a rotation in flight commits first and its successor gets revoked. +- O b6a9586: `Mint`/`Create` fall back to the context's consented admin for direct minting. +- V 3c53182 + D 054c15a (pointer bump): `/connect` sends `backend_admin` through the new `app/lib/consent.ts`. On a 409 it reloads the request and shows `connect.linkChanged` (en/pl). +- D a6be849: the demo e2e mint helpers bind backend grants to the linked admin, and the consent helper names the shown admin. +- J 521df9a: `TestJournalPromptWrites`'s fake grants now carry `BackendUserID`. The gate caught this: pier's fail-closed rule had broken the fixture. +**Tests:** +- F `TestToolsList` / "a backend grant acts only as the administrator it was consented for". Grants bound to admin 43, and to no admin, list no backend tool and cannot call it. +- F `TestFanOut` / "link change". A relink to another admin drops the backend subject; relinking the consented admin restores it. +- O `TestMCPBackendBinding`, 5 subtests: the decide 409 matrix; the binding travels code → exchange → rotation → verifier; direct mint honours the context; a link change deletes the backend codes so the exchange answers invalid_grant while a non-backend code still works; a revocation racing a rotation waits and revokes the successor. On the old code all four original subtests fail; the race subtest even hit "deadlock detected (SQLSTATE 40P01)". The direct-mint subtest fails without b6a9586. +- D `TestMCPEndToEnd` / "a grant consented for another administrator has no backend tools". +- V `app/lib/consent.test.ts` (5 tests). +- `TestMCPConsentSmoke` was updated to send `backend_admin`. + +### RC-WR-02: verifier ban refusal untested + +**Status:** fixed +**Commit:** O 89c9ec8 +**Applied fix:** the ban subtest now writes `user_throttle.is_banned` directly, checks that `tokens_valid_after` stays NULL, asserts both the verifier and the rotation refuse, and that unbanning restores both. A new subtest refuses the rotation of a token created before the cutoff and leaves its rows untouched. The doc comment and README were corrected. +**Tests:** `TestMCPVerifierRevocation` / "a ban alone refuses the verifier and the rotation until unbanned" and "the refresh rotation refuses a token created before the cutoff". Mutation checks: removing the verifier ban check fails at line 152; removing only the rotation ban branch fails at line 154; removing the rotation cutoff check fails at line 170. + +### RC-IN-01: link code accepted from the query string on POST routes + +**Status:** fixed (the nginx part is deferred) +**Commit:** U 92676ba +**Applied fix:** new unexported `readBodyFields` (a JSON body or `r.PostForm`, never the query string), used by `BackendLinkShowPost` and `BackendLinkConfirm`. README updated. +**Test:** `TestBackendLinkShowPost` / "a code in the query string is ignored". Show with `?code=` and no body, an empty JSON body or a form without the code gives 404; confirm with `?code=` gives 404; a form body still works. On the old code it fails ("= 200 ... want 404"). + +### RC-IN-02: the auth_time one-second blind spot + +**Status:** fixed (documented, comparison unchanged as the design accepts it) +**Commit:** U b929ba9. The doc comment on `ConfirmLinkRequest` and an inline note now state the blind spot and why it is accepted. + +### RC-IN-03: logo and avatar URL fallback ignored the public path + +**Status:** fixed +**Commit:** U 9730110. `OrganisationLogoURL` and `AvatarURL` fall back to `f.URL()` (attach.PublicURL). README updated. +**Tests:** `TestAvatarURL` (its assertion pinned the old behaviour and now asserts the public URL) and `TestOrganisationLogo` (new failing-bucket check). Both fail on the old code ("/fac/... want /storage/uploads/fac/..."). `TestAvatarURL` was added to the gate's USER_TESTS. + +### RC-IN-04: nginx /mcp cap equal to pier's + +**Status:** fixed +**Commit:** D 40ed518. `client_max_body_size 5m;` on `location = /mcp`; `scripts/check-deploy.sh` requires the new value. `check-deploy.sh` passes, and it refuses the old config. + +### RC-IN-05: test gaps + +**Status:** fixed +**Commits:** +- U ebe408f: `TestOrganisationLogo` asserts the blobs exist before the delete and that all keys under both partition directories (originals and thumbnails) are gone after the commit. A mutation (DeleteKeys disabled) fails ("blobs left after the delete"). The listed U Postgres tests now call `t.Skip("Postgres")` under -short. +- O 9813c1d: the same `t.Skip` change in `mcp_revocation_test.go`. +- D 09c3d31: the same change in `TestMCPRevocationEndToEnd`. +- V 9c247ab + D 036919c (pointer bump): `linkCode.test.ts` pins `#code=&next=/x` returning the code (the re-review's ruling) and documents it in `readLinkCode`. + +### RC-IN-06 (cheap part): MintAPIToken deprecation + +**Status:** partially fixed; the boot-order part is deferred +**Commit:** U 9892404. `MintAPIToken` is marked `Deprecated:` in favour of `MintAPITokenFor`, and the README says so. No internal caller is left; consuming apps still compile. + +### Gate + +F f824d93 `test(19.7)` adds `TestMCPBackendBinding` to OAS_TESTS and `TestAvatarURL` to USER_TESTS. + +## Skipped Issues + +### RC-IN-06 (boot-order part): fallback widened before the phrase-key check + +**File:** `U classes/api_token_scopes.go:125`, `U plugin.go:127-146` +**Reason:** deferred. Moving the add after the check needs either a new exported seam or a behaviour change to the exported `CompileAPITokenScopes` in a core plugin. A failed boot ends the process, so the widened set is never used in production. +**Original issue:** `CompileAPITokenScopes` widens the process-wide MintAPIToken fallback before boot checks the phrase keys. + +### RC-IN-07: oauthserver builds only through go.work + +**File:** `O go.mod:6` +**Reason:** deferred as a release-time item, per instructions. The requires were not bumped. The O release checklist must bump sm-user-plugin to the new U tag and pass `GOWORK=off go build ./...`. +**Original issue:** O uses U APIs that v0.1.10 lacks. + +### Optional parts deferred + +- RC-CR-01 optional: base the session cutoff on `auth_time`, and have `/refresh` and the jwt guard compare it with the cutoff. This changes session revocation in a core plugin and bouncer, and the gap predates the phase. +- RC-IN-01 optional: nginx `access_log off` (or a log_format without `$request_uri`) for `/_user/api/v1/backend-link/` until the deprecated GET route goes in a major release. This is a demo deploy change. + +All deferrals are recorded in F deferred-items.md (4d5a09a, 1fe553e). That file also marks the earlier `readLinkCode` entry resolved, and records that the demo's oauthserver working-tree checkout moved from 671513d to the canonical b6a9586 (detached, fast-forward, pointer uncommitted). The demo e2e tests need that checkout, because pier now requires the recorded consented admin. + +## Gate tail (bash scripts/check-phase19.7.sh --all, exit 0) + +``` +PASS go +PASS plugins +PASS consumers +PASS admin +PASS docs +PASS marker +PASS docker +Phase 19.7 gate passed WITH UNRELEASED PINS: the release pins are not yet proven (plugins/golem15/journal plugins/golem15/oauthserver plugins/golem15/user) +``` + +--- + +_Fixed: 2026-10-23_ +_Fixer: Claude (msd-code-fixer)_ +_Iteration: 1_