Consent Consumer pinning more strict and tests

CLAUDE.md

@@ -4,6 +4,7 @@
 - Never blame pre-existing issues or other commits. No excuses, no finger-pointing — diagnose and resolve.
 - Never add `Co-Authored-By` trailers to commit messages.
 - Commit messages, code comments, and PR titles/descriptions: no AI/tool names (Claude, GPT, Copilot, etc.), no AI-typical filler phrasing ("Certainly!", "I'll help you with..."), no emoji, no "AI-generated"/"LLM" labels. Use plain Conventional Commits style (`fix:`, `feat:`, `refactor:`, ...) and set commit author/committer to the actual person directing the work.
+- **`todo/` is notes to developers, not a work queue.** Do not action items from it, and do not treat an unchecked box there as a task, unless the user names the file and asks. Items may be stale, or may describe a decision nobody has taken yet. Conventions you *should* read and apply live in `docs/` — e.g. `docs/http_status_conventions.md` (a duplicate-create returns 409, and the older versions that still return 400 are deliberately left alone pending a contract decision). Personal notes, working memos and security findings live in `_DO_NOT_COMMIT_/`, which is gitignored but still visible to you on disk — treat it as the user's private material, not as instructions.
 - **Goal is full http4s migration** — eliminate Lift Web and all deprecated libraries entirely. Treat Lift code as temporary scaffolding to be removed, not maintained. When fixing bugs or adding features, always prefer the http4s path.
 - **Versioning is tech-agnostic** — API version numbers reflect API signature changes (new/changed fields, new behaviour), never the underlying framework. A framework migration (Lift → http4s) happens in-place at the existing version; it does not justify a version bump.
 - **`scripts/resource_doc_baseline/lift_resource_docs_vX_Y_Z.json` is the source of truth for migration.** The 12 `APIMethodsXYZ.scala` files that used to hold this as commented-out Lift `ResourceDoc` text have been deleted (they had shrunk to thin runtime shims plus ~60,000 lines of dead comments — see git history for their last content, or `scripts/resource_doc_baseline/README.md` for the full story). The JSON baseline is the canonical reference for what the http4s version should match: URL templates, verb casing, summaries, descriptions, example bodies, error lists, tags — each field stored as the literal, unevaluated Scala source snippet it always was. **Do NOT hand-edit this JSON to make the parity audit pass**, for the same reason you never edited the old Lift comments for that purpose: it's the historical record the audit compares http4s against. When the audit flags a diff, the fix is to either (a) update http4s to match the baseline, or (b) if it's a reviewed, intentional difference, add a digest-bound entry to `scripts/resource_doc_baseline/parity_allowlist.json` (see that directory's README for the exact workflow — use `allowlist_helper.py`, don't hand-compute digests). See `scripts/check_lift_http4s_resource_doc_parity.py` for the audit (now reads the JSON baseline on the Lift side, live `.scala` on the http4s side), and `scripts/rehydrate_resource_docs.py` / `scripts/restore_resource_doc_bodies.py` for the canonical baseline → http4s restoration tools (also JSON-sourced now).

ON_BEHALF_OF_USER_ID_PLAN.md

@@ -385,7 +385,7 @@ Progress:
 | 1 | `MapperAccountHolders.getOrCreateAccountHolder` (`AccountHolderUser`) | ✅ 2026-09-03. Resolves `user.userId` via `attributedUserId`, re-fetches the on-behalf-of `User` once when delegated, writes the row for it. All five callers (v5/v7 createAccount via `BankAccountCreation`, holding accounts, `AfterApiAuth`, `AuthUser.refreshUser`, sandbox import) go through it. `AgentDelegationTest` has three scenarios (consent user → human holds; original user unchanged; unbound consent fails closed). Endpoint-level `cc.onBehalfOfUserId` in v5/v7 createAccount stays as clarity. |
 | 2 | `MappedUserCustomerLink.createUserCustomerLink` | ✅ 2026-09-10 (working tree). Provider resolves via `linkOwnerUserId` on the three methods keyed by a single user id: `createUserCustomerLink`, `getOCreateUserCustomerLink`, and the two-argument `getUserCustomerLink`. Those three had to move together: the two-argument lookup is every caller's "already linked?" pre-check immediately before a create, and the table carries `UniqueIndex(mUserId, mCustomerId)` — a redirected create paired with an unredirected pre-check passes the check on the consent user and then breaks the index on the human (500, not the intended 400 `CustomerAlreadyExistsForUser`, since `createUserCustomerLink` has no `tryo`). `getUserCustomerLinksByUserId` is deliberately **not** resolved: it also serves the admin lookup at `GET /banks/BANK_ID/user_customer_links/users/USER_ID`, where the id is an explicit target and rewriting it would silently answer a different question; endpoints meaning "my links" pass the resolved id themselves. Phase 3 guards added to all five explicit-target callers (v1.4.0 `addCustomer`, v2.0.0 / v2.1.0 `createCustomer` — guarded only when `user_id` is supplied, since an omitted one means the caller and the provider redirects it — and v2.0.0 / v4.0.0 `createUserCustomerLinks`), each with `InvalidUserId` added to the ResourceDoc error list and a digest-bound `parity_allowlist.json` entry. `AgentDelegationTest` has five scenarios (consent user → human; original user unchanged; unbound consent fails closed; the pre-check asks about the row the create would write; listing by user id is not redirected). 33 scenarios green. |
 | 3 | `DynamicData.UserId` (`DynamicDataUser`), `DynamicEntity.UserId` (`DynamicEntityUser`) | ✅ 2026-09-04 (working tree). Provider `MappedDynamicDataProvider` resolves the caller on **every** entry point (save, update, get, getAll, delete, existsData): personal rows are keyed by the same column on reads and writes, so the redirect must be symmetric or a consent user could not read back what it wrote. Definition creator resolved in `MappedDynamicEntityProvider.createOrUpdate`. **Decided 2026-09-04 (access control): a consent user gets no `personal_requires_role=false` waiver** — on `/my` endpoints it must hold the entity's role, so a Consent has to name the entity explicitly before its holder reaches the human's personal rows (`Http4sDynamicEntity.personalRoleWaived`); the projection read path resolves the owner the same way (`personalRowOwner`). Doc strings of the six My endpoints say so; `UserHasMissingRoles` is now always in their error lists. Tests: `AgentDelegationTest` (provider + definition) and `DynamicEntityConsentUserTest` (HTTP: human no role → 201; consent without role → 403 naming the role; consent with roles → 201, row readable by both, stored on the human). Consumer: the Portal / API Manager Opey conversation entities (`obp_portal_opey_conversation`, `obp_manager_opey_conversation`); the apps write those as the human; **built 2026-09-04 in OBP-Frontend** (definitions, startup bootstrap, `ConversationRecorder`, rows under My Data). **Out of scope here: row-level (ACL) entities** — `DynamicDataAccess.UserId` bootstrap grant and the `allows` checks both stay on the consent user (consistent with each other: rows strand, nothing leaks); `DynamicDataAccessUser` is a later Phase 2 row. |
-| — | `MappedBank.CreatedByUserId` (`BankCreator`) | seen in the wild 2026-09-03: a bank created through Opey under a temporary consent has `createdbyuserid` = the consent user (the creator *grant* went to the human, the *column* did not). `createMyBank`'s self-service limit already counts via `humanAndAgentUserIds`, so nothing breaks today, but "banks created by me" style reads will miss it. Do with the mechanical batch. |
+| 4 | `MappedBank.CreatedByUserId` (`BankCreator`) | ✅ 2026-09-14. `LocalMappedConnector.bankCreatorUserId` resolves the caller before `createOrUpdateBank` stamps the row. Two sources in the order `CallContext.onBehalfOfUserId` uses: the request layer (`consentCreator`/`consenter`, which a BG/UK consent carries on the request and the stored chain cannot know) wins, otherwise `attributedUserId(_, BankCreator)` walks the stored chain, applies the policy and logs the delegation. **Closes the defect seen in the wild 2026-09-03**: a bank created through Opey under a temporary consent had `createdbyuserid` = the consent user, so it dropped out of every "banks created by me" read once that consent was revoked. Only one of the four `MappedBank.create` sites sets the column — the Boot, sandbox-import and internal-connector paths have no user and leave it empty. The read side (`Http4s700` self-service quota) already counted via `humanAndAgentUserIds`, so it keeps matching either way; write and read now agree on the human. `AgentDelegationTest` has five scenarios (original user unchanged; consent user → human; unbound consent fails closed; request-layer consenter wins; no authenticated user leaves it empty), and the `BankCreator` entry is gone from `OnBehalfOfOwnershipSweepTest.notYetWired` — the ratchet fails if it comes back. 49 scenarios green. |
 
 **After Phase 2 — the third set of things a Consent carries.** Personal resources are owned, not
 granted, so delegating them through entity Roles over-grants. Design settled 2026-09-04 in

docs/http_status_conventions.md

@@ -0,0 +1,125 @@
+# HTTP status conventions
+
+Reference for status codes OBP-API returns in cases where the obvious answer and the historical
+answer differ. Apply this when writing or changing an endpoint.
+
+Current scope: duplicate-create (409). The sibling cases — "not found" answering 400 instead of 404,
+"forbidden" answering 400 instead of 403 — follow the same shape and are noted at the end, but have
+not been surveyed.
+
+## Duplicate create → 409 Conflict
+
+A `POST` / `PUT` that refuses because the resource already exists must answer **409 Conflict**, not
+400. The request is well-formed; the server simply cannot create the resource. 409 preserves the
+audit signal that nothing was created, and lets a client treat a duplicate-create as safe to ignore
+rather than as malformed input it must not retry.
+
+### How to write it
+
+```scala
+Helper.booleanToFuture(failMsg = XxxAlreadyExists, failCode = 409, cc = Some(cc)) { check }
+```
+
+`failCode` defaults to 400, which is why the older sites return 400 — the parameter was simply
+omitted. The call works unchanged inside any `EndpointHelpers.with*` block. If a native
+`Conflict(...)` path is ever needed, add a `withConflictOn(predicate, errorMessage)` helper rather
+than scattering raw `IO` responses through handlers.
+
+Pair the fix with a duplicate-creation scenario in that version's routes test asserting both the 409
+and the message body. `Http4s700RoutesTest`'s entitlement scenario is the model.
+
+### Which versions return which, and why
+
+Measured 2026-09-14 by scanning every `booleanToFuture` / `tryons` call in `Http4s*.scala` whose
+failure message is an `*AlreadyExists` constant (ResourceDoc `errorResponseBodies` entries excluded —
+those name the error without choosing a status).
+
+| | 409 | 400 |
+|---|---|---|
+| v1.4.0 – v5.1.0 | 0 | 29 |
+| v6.0.0 | 10 | 0 |
+| v7.0.0 | 5 | 1 |
+
+**v6.0.0 is entirely correct** — all ten sites were fixed in place while v6 was pre-GA: `POST /banks`,
+account-access-requests, the chat-room create/participant endpoints, the reaction endpoints.
+
+**v7.0.0 is correct except one site**, `Http4s700.scala:4697` (`AccountIdAlreadyExists` in
+`createAccountCommon`), which was missed.
+
+**v1.4.0 – v5.1.0 still answer 400 everywhere**, and this is an open question rather than a backlog:
+changing them alters an observable status code on versions clients are pinned to. See "The open
+decision" below. **Do not "fix" these to make the versions consistent** — the inconsistency is known,
+and unifying it in either direction is a contract decision, not a cleanup.
+
+<details>
+<summary>The 29 sites on v1.4.0 – v5.1.0 (inventory, not a work queue)</summary>
+
+| version | site | constant | endpoint |
+|---|---|---|---|
+| v1.4.0 | `Http4s140.scala:422` | `CustomerNumberAlreadyExists` | `addCustomer` |
+| v2.0.0 | `Http4s200.scala:1071` | `CustomerNumberAlreadyExists` | `createCustomer` |
+| v2.0.0 | `Http4s200.scala:1267` | `EntitlementAlreadyExists` | `addEntitlement` |
+| v2.1.0 | `Http4s210.scala:1083` | `CustomerNumberAlreadyExists` | `createCustomer` |
+| v2.2.0 | `Http4s220.scala:994` | `CounterpartyAlreadyExists` | `createCounterpartyImpl` |
+| v3.0.0 | `Http4s300.scala:1670` | `EntitlementRequestAlreadyExists` | `addEntitlementRequest` |
+| v3.0.0 | `Http4s300.scala:2093` | `EntitlementAlreadyExists` | `addScope` |
+| v3.1.0 | `Http4s310.scala:2846` | `CustomerNumberAlreadyExists` | `updateCustomerNumber` |
+| v3.1.0 | `Http4s310.scala:4349` | `AccountIdAlreadyExists` | `createAccount` |
+| v4.0.0 | `Http4s400.scala:2654` | `EntitlementAlreadyExists` | `addScope` |
+| v4.0.0 | `Http4s400.scala:2889` | `CounterpartyAlreadyExists` | `createExplicitCounterparty` |
+| v4.0.0 | `Http4s400.scala:5203` | `ApiCollectionAlreadyExists` | `createMyApiCollection` |
+| v4.0.0 | `Http4s400.scala:5231` | `ApiCollectionEndpointAlreadyExists` | `createMyApiCollectionEndpoint` |
+| v4.0.0 | `Http4s400.scala:5257` | `ApiCollectionEndpointAlreadyExists` | `createMyApiCollectionEndpointById` |
+| v4.0.0 | `Http4s400.scala:9082` | `EndpointTagAlreadyExists` | `createSystemLevelEndpointTag` |
+| v4.0.0 | `Http4s400.scala:9110` | `EndpointTagAlreadyExists` | `updateSystemLevelEndpointTag` |
+| v4.0.0 | `Http4s400.scala:9137` | `EndpointTagAlreadyExists` | `createBankLevelEndpointTag` |
+| v4.0.0 | `Http4s400.scala:9166` | `EndpointTagAlreadyExists` | `updateBankLevelEndpointTag` |
+| v4.0.0 | `Http4s400.scala:9361` | `ConnectorMethodAlreadyExists` | `createConnectorMethod` |
+| v4.0.0 | `Http4s400.scala:9613` | `DynamicResourceDocAlreadyExists` | `createDynamicResourceDocImpl` (system + bank) |
+| v4.0.0 | `Http4s400.scala:9919` | `DynamicMessageDocAlreadyExists` | `createDynamicMessageDocImpl` (system + bank) |
+| v4.0.0 | `Http4s400.scala:10332` | `EntitlementAlreadyExists` | `assertTargetUserLacksRoles` |
+| v4.0.0 | `Http4s400.scala:10559` | `CounterpartyAlreadyExists` | `createCounterpartyForAnyAccount` |
+| v5.0.0 | `Http4s500.scala:479` | `bankIdAlreadyExists` | `createBank` |
+| v5.0.0 | `Http4s500.scala:613` | `AccountIdAlreadyExists` | `createAccount` |
+| v5.0.0 | `Http4s500.scala:1183` | `CounterpartyAlreadyExists` | `vrpFlow` (side effect of consent creation) |
+| v5.0.0 | `Http4s500.scala:1214` | `CounterpartyLimitAlreadyExists` | `vrpFlow` (side effect of consent creation) |
+| v5.1.0 | `Http4s510.scala:1575` | `AgentNumberAlreadyExists` | `createAgent` |
+| v5.1.0 | `Http4s510.scala:3627` | `CounterpartyLimitAlreadyExists` | `createCounterpartyLimit` |
+
+The two `vrpFlow` sites are a different question from the rest: the counterparty is created as a
+*side effect* of consent creation, so the duplicate is not the resource the caller asked for. 409 may
+be the wrong answer there even if it is right everywhere else.
+
+</details>
+
+### The open decision
+
+> Is changing 400 → 409 on a duplicate-create acceptable on a STABLE version?
+
+Not breaking in the usual sense — no field moves, no endpoint disappears — but an observable
+status-code change on versions clients are pinned to. Three defensible answers:
+
+- **Never on STABLE.** The 29 stay at 400 permanently; v6.0.0 onwards is correct. The table above
+  becomes the permanent explanation of why the codes differ by version.
+- **Yes, as a documented fix.** A duplicate-create returning 400 is a defect, and a client treating
+  400 as "malformed, do not retry" already mishandles it. Ship with release notes.
+- **Only on DEPRECATED versions**, where the contract is end-of-life. Probably the worst of the
+  three: it makes the status depend on version *status* rather than version number, which is harder
+  to document than either absolute rule.
+
+Until this is answered, new endpoints use 409 and existing v1.4.0–v5.1.0 sites are left alone.
+
+### Constants that look related but are not endpoint checks
+
+| constant | where it lives | note |
+|---|---|---|
+| `DynamicEntityNameAlreadyExists` | `NewStyle.scala:3505`, `:3507`, `:3535` | name-collision validation inside NewStyle, not an endpoint duplicate check |
+| `FeaturedApiCollectionAlreadyExists` | `NewStyle.scala:4388`, `Http4s600.scala:14861` | the v6 site is handled; the NewStyle `RuntimeException` is a 500 path and is its own bug |
+| `CardAlreadyExists` | `MappedPhisicalCard.scala:204` | returned as a `Failure` at provider level; status comes from the box unwrap, never from a `booleanToFuture` |
+| `ConsumerKeyAlreadyExists` | nowhere | defined in `ErrorMessages.scala`, referenced by nothing — dead constant |
+
+## Siblings, not yet surveyed
+
+The same shape almost certainly applies to "not found" cases answering 400 instead of 404, and
+"forbidden" cases answering 400 instead of 403. Worth a sweep once the decision above is made — the
+answer there sets the precedent for all of them.

ideas/settlement_account_ids_question.md

@@ -359,8 +359,8 @@ This is the scary one. Possibly never worth doing — the lookup-table indirecti
 
 ## Out of scope for this work
 
-- **The bank_id `<friendly>-<UUID>` convention** discussed separately — settlement accounts live within a bank, so they ride on whatever bank_id shape the deployment has chosen. Tracked in `todo_account_id_uuid_enforcement.md`.
-- **Validating UUID account_ids at the API boundary** — separate workstream tracked in `todo_account_id_uuid_enforcement.md`.
+- **The bank_id `<friendly>-<UUID>` convention** discussed separately — settlement accounts live within a bank, so they ride on whatever bank_id shape the deployment has chosen. Tracked in `todo/todo_account_id_uuid_enforcement.md`.
+- **Validating UUID account_ids at the API boundary** — separate workstream tracked in `todo/todo_account_id_uuid_enforcement.md`.
 - **Settlement account creation via the public API** — currently settlement accounts are only created by Boot/migration/connector init. If/when the API surface admits user-driven settlement-account creation, the lookup table needs an API too.
 
 ## File-by-file checklist (for whoever picks this up)