019fb836 DfxApproval-GSheets durch API-Workflow ersetzen - #4553
Merged
Conversation
TaprootFreak
force-pushed
the
codex/dfxapproval-api-workflow
branch
from
August 1, 2026 00:52
18f69f6 to
d05a9d7
Compare
TaprootFreak
marked this pull request as ready for review
August 1, 2026 00:58
Collaborator
Author
|
Verification state after the review corrections:
Three points in the PR description need an operator decision before the cutover. |
Collaborator
Author
|
Five review passes were needed before two independent reviews came back with no findings. What the passes changed, beyond the corrections already listed in the description:
Two reported points were deliberately not applied, with the reasoning recorded in the pull request: |
The overlay text was drawn with a PDF standard font, which is limited to WinAnsi and throws on every character outside Latin-1. Names, streets and employers in Polish, Turkish, Baltic, Czech or Cyrillic spelling therefore failed the whole document, leaving the case without the evidence its approval depends on - silently, once per minute. The text is now set in an embedded Liberation Sans, which is metrically compatible with Arial/Helvetica, so the layout is unchanged. Only the glyphs actually used are embedded (about 9 kB per document). Characters the font has no glyph for, such as CJK, are substituted instead of failing the document.
The reuse path fetched the whole blob only to find out whether it exists, and discarded it. It now lists the key instead and fails loudly when a document that the database marks valid is gone from storage. It also exposes the registered file of a generation key, so a retry after a failed upload can keep the name of the first attempt - that same string is printed into the document as its document number, so name and content must not drift apart.
Two loads in the workflow could not work against a real database, and the mocked tests hid it: - The completion loaded the step without its user data relation. `KycStep.userData` is a ManyToOne without eager loading, so it stayed undefined and reading its ID threw for every case. The step is now loaded through a query builder that joins the user data and locks the step alone. - The risk-data initialisation locked user data with a plain find. That joins the eager, nullable relations of UserData, and Postgres rejects FOR UPDATE on the nullable side of an outer join. It now locks through a query builder without those joins. Both lock aliases are lower case on purpose: the driver emits FOR UPDATE OF <alias> unquoted and Postgres folds it to lower case, so a camelCase alias is not found in the FROM clause. Beyond the two loads: - RiskProfile and FormA follow the account, not a KYC step. Requiring a DfxApproval or FinancialData step as an anchor silently skipped every account without one, which the productive Sheet covers. Both documents are now generated from user data alone, serialised on a user-data advisory lock. - Missing NameCheck evidence only fails the NameCheck document instead of the whole case. - The gate blockers are logged instead of discarded: a case waiting forever was indistinguishable from a case nobody looked at. - The excluded account IDs move from the source tree into the `dfxApprovalDocumentExclusions` setting, and the level log keeps the wording of the existing approval path.
Records the embedded Unicode font, the account-bound selection of RiskProfile and FormA, the exclusion setting that has to be filled before the cutover, and the gate-blocker log line as a monitoring source.
Every other document under docs/ is in English, and this repository is public. Same content, plus the operator account is no longer named explicitly.
Three points from the review round: - The batch that initialises empty risk fields passed `hasOpenNameChecks = false` instead of asking. A new sanctioned hit does not refresh `lastNameCheckDate` - that only happens on a clean result - so a still-valid date is no proof that no check is open. Such an account could get `highRisk = false` written, and a RiskProfile stating exactly that, while a sanctions match was still unresolved. The flag is now queried per account. - RiskProfile and FormA were reachable from two different advisory-lock namespaces: the step lock in the approval loop and the user-data lock in the account-bound batch. Two instances could therefore render and upload the same document concurrently under the same generation key, each with its own embedded timestamp. Both documents now belong to the account-bound path alone. - The approval audit entry recorded only step status, KYC level and KYC status, while the same transaction also overwrites `complexOrgStructure`, `highRisk`, `depositLimit` and `amlAccountType`. A limit set by Compliance was therefore not recoverable from the database. All four previous values are now part of the entry, which is written before the columns change. The log entries and the step completion now go through the existing `KycLogService.createLogInternal` and `KycStep.complete()` instead of rebuilding both by hand.
…ntory - The unique index on `kyc_file.generationKey` carried a hand-written name, which the contribution guide rules out twice. Entity and migration now use the name TypeORM generates itself, and the constraint check in the storage service refers to the same constant. - The new minute-interval job was missing from the cron inventory the contribution guide requires. Added, with the counts recomputed the way that document describes: 132 declarations across 93 files, 111 of them with a flag, 50 on a minute interval.
CustomerProfile had the same cross-lock gap that RiskProfile and FormA had: the approval loop requested it under the lock of the DfxApproval step while the batch generated it under the lock of the FinancialData step. The two candidate sets overlap exactly at `kycLevel = 40`, so two instances could render and upload the same document concurrently, each with its own embedded document number. Each document now has a single path and a single lock key: GwGFileCover, IdentificationForm and DfxNameCheck under the DfxApproval step lock, CustomerProfile under the FinancialData step lock, RiskProfile and FormA under the user-data lock. That makes the combined selection helper redundant. Also: the module imports are back in alphabetical order, and the comment on the exclusion setting now says what the setting actually affects.
The contribution guide asks for alphabetically sorted imports. Moving the status DTO out of the `output` directory and adding the fontkit import had left four import blocks out of order.
Applies the repository's import order - non-relative paths first, then relative ones, each alphabetically - to all files this pull request touches, not just the ones spotted so far.
Three properties of the Sheet process are reproduced as they are - the compliance defaults written on approval, the RiskProfile requirement outside a FATF-enabled country, and skipping documents that already exist. Each is a property of the process being replaced, not of this implementation, and changing any of them would change the outcome rather than the mechanism. The residual risk is stated as well: the selection rules of the six document sheets come from the audit package rather than from their source, because the script export of those projects is refused.
@DfxCron now requires scope; the workflow writes KYC records and belongs on the worker, not the API process.
develop's download path still uses getBlob; the generated-file helper uses listKeys. The suite needs both spies after the rebase.
TaprootFreak
force-pushed
the
codex/dfxapproval-api-workflow
branch
from
August 13, 2026 21:24
a505cb1 to
9d3f340
Compare
Prettier failed CI on docs/cron-jobs.md after the rebase edit.
TaprootFreak
marked this pull request as draft
August 13, 2026 21:54
CONTRIBUTING requires every touched file at 100% on all four metrics and a ratchet pin in the same PR. The new workflow, PDF, check, document, catalog and module files now have specs that reach that bar; the cutover flag and advisory-lock fakes are declared.
Map personal source_of_funds onto the seven CustomerProfile categories, treat unknown PEP as not-no, generate RiskProfile only when pep is false, reuse a valid generation-key winner, reload account data under the user-data lock, and keep only the newest FinancialData step per customer profile.
FinancialData stores multiple-choice answers as a comma-separated list. Split before ticking the template categories, and assert the risk-init query uses the 90-day lastNameCheckDate window.
The gate result is never exposed as an HTTP contract, so the repo rule is interface rather than class.
Skip already-complete files so the 500-row window cannot starve older cases, lock UserData inside the approval transaction, and look up generation keys through KycFileService instead of a forwarding wrapper.
Drop the pre-filter row cap so already-complete cases cannot hide later work, keep the newest FinancialData step per account, and mark a leftover WORM object valid instead of uploading it again.
Empty compliance values must not be drawn as no. Sort the KycFile entity spec imports alphabetically.
A known true value still needs the document so Compliance can see it. Only an unset flag stays out of generation.
The document key is per user. Two FinancialData sequences of the same account must not write it under two step lock keys.
The generation key is per account. The FinancialData step is only the selection source, not the lock key.
A hit that lands between the pre-check and the update must not still receive the empty-field defaults.
Default the exists double to false so the write path stays reachable, and pin both the stale-date and late-hit cases.
Extract the in-lock skip condition so both a stale name-check date and a late sanctioned hit are asserted, including the empty update after another worker already filled the columns.
Record the step comment before clearing it. Generate FormA and RiskProfile only after verifiedName is set, and IdentificationForm only after identificationType is set, so a later name or type cannot auto-approve an empty first render.
Do not render or freeze a DfxApproval PDF that would silently omit the verified name or default an unknown identification type to OnlineId. The approval gate treats whitespace-only names as missing.
Match the existing KYC jobs instead of rolling date arithmetic.
The lock double and storage mock drifted, and getByGenerationKey took the line that still named getUserDataKycFiles.
The workflow and document services add ten database reads that the published inventory did not list. Rows and widths come from the inventory scan of this head; advisory locks stay out of the table.
Permanently not-ready MANUAL_REVIEW steps no longer occupy the oldest-50 window: the review query orders by updated and a not-ready evaluation bumps that timestamp. Advisory-lock unlock failures still release the query runner. Document generation now selects candidate ids instead of the full user graph, and getByGenerationKey no longer loads eager relations.
A sanctioned name check, a thrown PDF/storage error, or a failed CustomerProfile render no longer pins the same 50/500 ids at the front of the next cron tick. Test-reality line numbers follow the spec.
The migration contract substitutes characters the font cannot encode instead of aborting the document, and those substitutions must be visible per case.
Collaborator
Author
|
EN: DE: DetailsReview: Grok quality + logic PASS_CLEAN, then Codex quality + logic PASS_CLEAN, both STATUS complete at Comments: paginated issue comments (2 historical author notes, no open request), reviews 0, inline 0, unresolved review threads 0. Mergeable: CI at |
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
1 participant
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.
EN:
This pull request replaces the eight live personal DfxApproval Google Sheets with a default-off API workflow.
The minute job generates the six compliance PDFs, rotates blocked cases out of the review window, and completes personal KYC only when the gate is clear.
It stays inert unless KYC_DFX_APPROVAL_WORKFLOW_ENABLED is exactly true and the KYC_DFX_APPROVAL process flag is on.
Every touched production file is pinned at 100 percent coverage.
DE:
Dieser Pull Request ersetzt die acht produktiven persönlichen DfxApproval-Google-Sheets durch einen default-off API-Workflow.
Der Minutenjob erzeugt die sechs Compliance-PDFs, rotiert blockierte Fälle aus dem Review-Fenster und schliesst persönliches KYC nur bei freiem Gate ab.
Ohne KYC_DFX_APPROVAL_WORKFLOW_ENABLED genau true und ohne Process-Flag KYC_DFX_APPROVAL bleibt er inert.
Jede angefasste Produktionsdatei ist auf 100 Prozent Coverage gepinnt.
Details
Replaces the personal DfxApproval sheets (approval, risk flags, GwGFileCover, IdentificationForm, CustomerProfile, RiskProfile, FormA, DfxNameCheck). IdentReport and PersonalNameCheck stay in their existing KYC processes. Organizations are out of scope.
Default-off:
KYC_DFX_APPROVAL_WORKFLOW_ENABLED === 'true'andProcess.KYC_DFX_APPROVAL. Advisory locks are session-scoped (pg_try_advisory_lock), not transactional. Document generation loads candidate ids, not full user graphs. Permanently not-ready review steps, skipped risk-init accounts and failed CustomerProfile renders bumpupdatedso they do not occupy the oldest-50/500 window. Characters the embedded font cannot encode are substituted and logged once per document.No public API contract change. Migration adds nullable unique
generationKeyonkyc_file. Cron inventory, test-reality fakes and load-sites are updated in this PR.