Repository navigation
feat(legal-status): global append-only store for public legal status (ADR 0006) - #13
Merged
Merged
Conversation
…(ADR 0006) Legal status of a published patent is public and the same for every organization, so it is stored once in two global tables without organization_id or tenant RLS. Which organization looks at which patent is sensitive, so no requester, case or user field is allowed in these tables; tenant references will live in tenant-scoped (FORCE RLS) tables. - 0018_legal_status_store: legal_status_source_records (each lookup, including not_found, with request_ref, retrieved_at, the source's own data time, the rule-relevant extract, and the SHA-256, size and storage key of the exact response bytes) and legal_status_assessments (status, term, countries, evidence, review reasons, rules_version, content_sha256; unique per record/as_of/rules_version). Runtime roles: worker SELECT+INSERT, app SELECT, platform nothing; no UPDATE/DELETE, so both are append-only. - Raw responses are content-addressed in object storage (public/legal-status/<source>/<sha256>.json). The DB keeps only fields the rules read. Raw retention involves public personal data and needs compliance sign-off (ADR 0006). - Worker LegalStatusRefresher: fetch -> archive exact bytes -> record -> assess from the stored extract -> record. A source failure writes nothing; not_found records the lookup but no status. - Clients now expose the exact response bytes (SourceResponse) so the stored hash proves what the source sent. - RULES_VERSION and canonical serialization in modules/legal_status. Tests: 8 Postgres integration tests (exact grants, no tenant columns, app cannot write, worker cannot UPDATE/DELETE, found rows must carry extract and hash, US/EP end-to-end with each stored assessment reproduced from its stored extract to the same content hash, not_found, failure writes nothing) plus 3 unit tests. test-postgres.sh now runs the new file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
corlin
added a commit
that referenced
this pull request
Oct 4, 2026
…ation bug they exposed (#15) ## Why `scripts/test-postgres.sh` ran a hand-kept list of **5 of the 14** integration files, and CI skipped every Postgres test. The other 9 had **never run**. Running them exposed one real product bug, plus stale tests and docs. ## Product bug found: case creation failed under the real DB roles (`b9905b9`) `CaseService.create_case` locked the quota row with `SELECT … FOR UPDATE`. PostgreSQL row locks need `UPDATE` privilege, and `patent_evidence_app` only has `SELECT` on `organization_plan_quotas`. So **every case creation failed** (`permission denied for table organization_plan_quotas`) in any deployment using the intended roles. Unit tests mock the DB, and the integration test for this path had never run. **Fix:** serialize creations with the organization advisory lock (`pg_advisory_xact_lock`; re-entrant, and `access.mutation` already takes it) and read the quota without a row lock. This prevents concurrent over-allocation **without** granting the API role `UPDATE` on quotas. It's covered by `test_case_lifecycle_and_quota_enforcement`, which now runs (quota of 2 preserved). ## Integration suite repaired (`9480995`) What was wrong: - **6 business-flow tests** (cases/documents, features, search, comparison, reports, full e2e) read `PE_MIGRATION_DATABASE_URL`, which nothing exports, so they always skipped. Their seeds targeted a schema the repo never had: - a non-existent `credentials` table - `organizations.persisted_status` - `memberships.identity_id` - missing `display_name` (NOT NULL since the first schema commit) - positional calls to the keyword-only `login_with_totp` - **`test_p0_02_e2e_lifecycle`** truncated tables that don't exist, called the bootstrap script with an old interface, and expected stale keys and status codes. - **`test_database_context`** depended on data seeded by another file, so it passed only in one file order. What changed: - **The script runs the whole `integration/` directory**, so files can no longer silently be left out. **CI now runs the script.** - `test_support` has shared helpers written against the real schema: - `reset_database` (derived from the catalog, so it can't go stale) - seed helpers for tenants and platform admins - `login_with_existing_totp` for repeat logins - `api_settings` now sets `worker_database_url` (the worker had been connecting with default credentials) - **E2E expectations aligned with the implemented contracts.** Each one was checked against the API code, the web client or a passing test before the test was changed: - provisioning needs `Idempotency-Key`, `plan_key` and a period - `invitation_token` is top-level - accept returns 201 - TOTP confirm returns `"confirmed"` - `organization.status` is the effective status - the handoff path is per strategy **Result: 91 passed, 0 skipped.** Before: 84 passed, with 9 files never executed. ## Design decision for review: a member of a suspended organization gets 403, not 404 The e2e test expected a "safe 404", but it never actually exercised suspension: - It skipped the MFA challenge, so the request stopped at 401. - With MFA added, it then hit the login rate limiter (4 per minute, frozen clock). Once fixed, the API returns **403 `forbidden`**, so suspension is enforced. This matches `OrganizationAccess`: **404 for non-members** (hiding that the organization exists) and **403 for members** of a non-active organization (who already know it exists). I aligned the test with that rule. If you want suspended members to get 404 as well, that's a product change, not a test fix. ## Docs and types (`97fa7c7`) - `runbook-platform-bootstrap.md` described an older script. It now matches the real one: positional email and display name, `PATENT_EVIDENCE_*` variables, **no credential material printed**, and a **30-minute** handoff (not 10). `local-development.md` was already correct. - Web `TotpConfirmResult.status` was typed `'verified'`, but confirm returns `'confirmed'`. No UI code branched on it; there are now separate types per endpoint. ## Correction to earlier PRs #6 said its integration test covered the upload checks. It never ran. **It now runs and passes**, so those checks are verified for the first time. ## Merge note #13 adds one file to the old 5-file list in `test-postgres.sh`. This PR replaces the list with the whole directory, so if they conflict, keep this PR's version; #13's tests are picked up automatically. ## Validation - `./scripts/test-postgres.sh`: **91 passed**, including the downgrade/upgrade cycle - `.venv/bin/pytest apps/api/tests/unit/`: 373 passed - `pnpm test:web`: 62 passed; `pnpm build:web`: ok - `verify-source-lock`, `verify-scaffold`, `verify-p0-02`, `verify-p0-e2e`, `docker compose config`: ok 🤖 Generated with [Claude Code](https://claude.com/claude-code)
corlin
added a commit
that referenced
this pull request
Oct 4, 2026
#17) …(ADR 0006) (#13) > Stacked on #11. Merge #11 first; this diff is storage-only. ## Design (ADR 0006) Legal status of a published patent is **public and identical for every organization**. It's stored once, in two **global** tables with no `organization_id` and no tenant RLS. This follows the repo's existing global-table pattern (`global_identities`), protected by exact grants. The integration suite requires RLS only on tenant tables. **Which organization looks at which patent is itself sensitive** (it reveals product direction and competitors). So these tables may contain no requester, case or user field; a test asserts this. Tenant references will live in tenant-scoped FORCE-RLS tables and bind `assessment_id` plus `content_sha256` at sealing time. | Table | Contents | |---|---| | `legal_status_source_records` | One row per lookup, **including `not_found`**: `request_ref` (no credentials), `retrieved_at`, the source's own data time (ODP `lastIngestionDateTime`), the rule-relevant `extract`, and the SHA-256, size and storage key of the **exact response bytes** | | `legal_status_assessments` | status, term, countries, evidence, review reasons, `rules_version`, `content_sha256`; unique per `(source_record_id, as_of, rules_version)` | | Role | Grants | |---|---| | `patent_evidence_worker` | SELECT, INSERT | | `patent_evidence_app` | SELECT | | `patent_evidence_platform` | none | There are no UPDATE/DELETE grants, so both tables are **append-only**: a new lookup or a new rules version adds rows and never rewrites history. - **Raw responses** are content-addressed in object storage (`public/legal-status/<source>/<sha256>.json`). The DB keeps only the fields the rules read. - **Worker flow** (`LegalStatusRefresher`): fetch → archive exact bytes → record → **assess from the stored extract** (so any stored assessment can be reproduced from the DB) → record. A source failure writes **nothing**. `not_found` records the lookup but no status. ## Needs a decision - **Retention of raw responses.** They contain public personal data (inventors, attorneys). Retention is a PIPL/GDPR question for compliance. Until it's decided, raw responses go to object storage only, under a prefix that can be deleted wholesale. - **Default refresh interval.** Not set yet; it should be measured from a first full run's quota use. - **Not in this PR:** job queue and triggering, tenant reference table, API read endpoints. - **`source_data_as_of` timezone:** ODP gives no UTC offset, so UTC is *assumed* for the column. The verbatim string in `extract` is authoritative. ## Integration test coverage gap found while doing this (pre-existing, not fixed here) `scripts/test-postgres.sh` runs only **5 of the 14** integration files, and CI skips all Postgres tests. When I ran all 14 against Postgres: - **6 business-flow tests** (cases/documents, features, search, comparison, reports, full e2e) are skipped even under the script: they read `PE_MIGRATION_DATABASE_URL`, which the script doesn't export. With it set, **all 6 fail at seeding**: `global_identities.display_name` is NOT NULL, and their seed data omits it. - `test_p0_02_e2e_lifecycle` errors on a non-existent table, `organization_audit_events`. - 3 `test_database_context` tests fail when run after other files: an FK error, i.e. an order or seed dependency. **Correction:** #6 said its lifecycle test covered the upload checks. Those tests never actually ran. I've added only this PR's new file to the script. Repairing the rest should be its own PR, since it may surface real regressions. ## Validation - `./scripts/test-postgres.sh`: **92 passed** (84 plus the 8 new), including the downgrade/upgrade cycle through `0018` - `.venv/bin/pytest apps/api/tests/unit/`: 376 passed - `pnpm test:web`: 62 passed; `pnpm build:web`: ok - `verify-source-lock`, `verify-scaffold`, `verify-p0-02`, `verify-p0-e2e`, `docker compose config`: ok 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Design (ADR 0006)
Legal status of a published patent is public and identical for every organization. It's stored once, in two global tables with no
organization_idand no tenant RLS. This follows the repo's existing global-table pattern (global_identities), protected by exact grants. The integration suite requires RLS only on tenant tables.Which organization looks at which patent is itself sensitive (it reveals product direction and competitors). So these tables may contain no requester, case or user field; a test asserts this. Tenant references will live in tenant-scoped FORCE-RLS tables and bind
assessment_idpluscontent_sha256at sealing time.legal_status_source_recordsnot_found:request_ref(no credentials),retrieved_at, the source's own data time (ODPlastIngestionDateTime), the rule-relevantextract, and the SHA-256, size and storage key of the exact response byteslegal_status_assessmentsrules_version,content_sha256; unique per(source_record_id, as_of, rules_version)patent_evidence_workerpatent_evidence_apppatent_evidence_platformThere are no UPDATE/DELETE grants, so both tables are append-only: a new lookup or a new rules version adds rows and never rewrites history.
public/legal-status/<source>/<sha256>.json). The DB keeps only the fields the rules read.LegalStatusRefresher): fetch → archive exact bytes → record → assess from the stored extract (so any stored assessment can be reproduced from the DB) → record. A source failure writes nothing.not_foundrecords the lookup but no status.Needs a decision
source_data_as_oftimezone: ODP gives no UTC offset, so UTC is assumed for the column. The verbatim string inextractis authoritative.Integration test coverage gap found while doing this (pre-existing, not fixed here)
scripts/test-postgres.shruns only 5 of the 14 integration files, and CI skips all Postgres tests. When I ran all 14 against Postgres:PE_MIGRATION_DATABASE_URL, which the script doesn't export. With it set, all 6 fail at seeding:global_identities.display_nameis NOT NULL, and their seed data omits it.test_p0_02_e2e_lifecycleerrors on a non-existent table,organization_audit_events.test_database_contexttests fail when run after other files: an FK error, i.e. an order or seed dependency.Correction: #6 said its lifecycle test covered the upload checks. Those tests never actually ran. I've added only this PR's new file to the script. Repairing the rest should be its own PR, since it may surface real regressions.
Validation
./scripts/test-postgres.sh: 92 passed (84 plus the 8 new), including the downgrade/upgrade cycle through0018.venv/bin/pytest apps/api/tests/unit/: 376 passedpnpm test:web: 62 passed;pnpm build:web: okverify-source-lock,verify-scaffold,verify-p0-02,verify-p0-e2e,docker compose config: ok🤖 Generated with Claude Code