fix(mercury): enforce Legal Person identity in private Finance account preflight

[chitcommit]

Contract defect

docs/contracts/mercury-multitenant.md requires every Mercury account to be attributable to exactly one Legal Person ChittyID. Finance PR #205 verifies exact Mercury organization, account and canonical tenant identity but omitted the existing Legal Person guard. A provider org/account match without Legal Person provenance must NOT open the new reconciliation consumer.

Bounded remediation

• Reuse server/lib/legal-person-binding.ts::isLegalPersonChittyId to validate the pre-existing accounts.metadata.legal_person_chittyid before the private Finance /mercury/account-binding route returns a Finance account UUID.
• Return 409 legal_person_binding_missing when absent/invalid/non-Person, without disclosing identifiers, creating records, assigning entity ownership or mutating books.
• Preserve the seven legacy webhooks and their existing flag-based transitional behavior; applies only to the new governed private read preflight.
• Add negative tests with deliberate absent, malformed and non-Person bindings.

Release gate

Read-only Neon audit 2026-10-12 showed zero of 42 Mercury-labeled accounts with legal_person_chittyid and zero explicit provider org binding; thus this safely remains fail-closed until independently sourced and operator-gated metadata corrections. ChittyConnect #343 and Finance #205 are deployed but Finance readiness/consume is still missing. Do NOT invent a ready/processed receipt. No new infrastructure or financial write.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1467bf3f-97ee-4ee7-9a53-162d746f078b

---

📥 Commits

Reviewing files that changed from the base of the PR and between f424f10 and 60aee62.

---

📒 Files selected for processing (2)

• server/__tests__/internal-chittyconnect.test.ts
• server/routes/internal-chittyconnect.ts

---

 ___________________________________________________
< Great artists ship, but not before a code review. >
 ---------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

---

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review: PR #206 (Legal Person guard on Mercury account-binding preflight)

Verdict: LGTM. I found no blocking issues.

Correctness

• The guard sits after the org-provenance checks and before the account-UUID check. Every failure path returns before a Finance account id can be returned.
• It reuses isLegalPersonChittyId, which is the single source of truth. That function requires a string and T = P. The regex matches the test ID 26-A-LGL-0001-P-10-C-Z.
• The 409 body is { error: 'legal_person_binding_missing' } with no identifiers. This meets the no-disclosure requirement.
• Only this new private read route changes. The seven legacy webhooks are untouched.

Suggestions (non-blocking)

1. server/lib/legal-person-binding.ts already exports getLegalPersonChittyId(account). It reads the same metadata key. fields.legal_person_chittyid is a second hard-coded read of that key. The module header says it is meant to be the single read surface, so a future move to a dedicated column (Path A) would then be a one-line swap. Using LEGAL_PERSON_METADATA_KEY or the getter here would keep that true. I haven't checked whether the getter also validates the format, so confirm that before switching.
2. Check ordering. An account with both a missing org binding and a missing Legal Person ID returns provider_organization_* first. That seems fine, and it avoids revealing which check failed. A short comment saying the ordering is intentional would help.

Tests

• Coverage is good. The happy-path fixtures now carry the binding, and there are negative cases for absent, non-Person (T), malformed (APTA) and empty-string values.
• The createTransaction undefined assertion is weak. It checks the mock's shape and not that nothing was written. Asserting that no mutating storage method was called would be stronger. It's minor, since the route only reads.
• A case where legal_person_chittyid is a non-string (number or null) would exercise the typeof branch. It is optional.

Operational note

• The PR description says 0 of 42 Mercury accounts currently carry the binding. This route will therefore return 409 for every account until operator-gated metadata is backfilled. That is the stated fail-closed intent. Make sure downstream consumers (ChittyConnect) treat 409 legal_person_binding_missing as a retryable "not ready" state and not as a hard failure.

Security and performance

• No concerns. The change is a pure in-memory check and adds no queries.

🤖 Generated with Claude Code

[claude]

Review

Small, well-scoped fail-closed guard. It reuses isLegalPersonChittyId/getLegalPersonChittyId as intended and does not touch the legacy webhooks. The error body discloses no identifiers. The guard runs after the tenant, account and org checks, so the 409 only appears once the other bindings match.

Notes / suggestions (non-blocking)

1. Validator is T=P only, not "Legal". isLegalPersonChittyId accepts any ...-P-... ID. Its own docs say it relies on upstream to vouch for the Legal subtype. The PR title and the 409 name say "Legal Person", so a natural Person ID will pass. If that's acceptable per the contract, fine. Otherwise this guard is weaker than the description implies. Worth a comment in the code.
2. Case sensitivity. The regex is uppercase-only. A lowercase-stored ID will fail closed, which is safe, but it makes the 409 look like "missing" when it's really "malformed". A distinct reason code would help operators during the metadata cleanup. Since the PR says 0/42 accounts have the field, that cleanup is coming.
3. Test coverage.
   • The malformed-ID test loops over '', 'APTA' and a T-type ID. Add a non-string value (e.g. number or null) and an array/object metadata case, since getLegalPersonChittyId handles those.
   • Add a test that the check is ordered after the org-provenance check, so a missing org binding still returns provider_organization_binding_conflict.
   • expect(storage.createTransaction).toBeUndefined() is vacuous. It checks the mock lacks a method, not that nothing was called. Consider a spy on the write methods, with not.toHaveBeenCalled().
4. Style. The fixture lines in the diff exceed the surrounding line width. Consider a shared bindingRow() helper to remove the repeated row literals.
5. Operational. Since all 42 current Mercury accounts lack the binding, the consumer will be 409 for every account until backfill. That matches the stated release gate. Make sure ChittyConnect treats legal_person_binding_missing as a terminal non-retryable state, or alerting/retry loops could get noisy.

No security or performance concerns. I didn't run the tests. LGTM once point 1 is confirmed as intended.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-12T06:39:11.455473Z	60aee62	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 60aee62ed0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Verify the Legal Person instead of only its type code

When legal_person_chittyid contains any syntactically valid T=P identifier for a natural or synthetic person, this guard passes: isLegalPersonChittyId explicitly checks only group 5 and its own documentation notes that the Legal subtype is not encoded in the ID. Because this route neither receives an expected Legal Person ID from the broker nor verifies the stored value with the delegated ChittyID authority, malformed ownership metadata can still open reconciliation for an account that is not bound to a verified P-Legal entity. Require an authoritative subtype/identity assertion or compare against a broker-supplied, verified Legal Person identity.

AGENTS.md reference: AGENTS.md:L90-L92

Useful? React with 👍 / 👎.