Release: develop -> main - #4413
Merged
Merged
Conversation
* feat(bank): add a receive-IBAN check for the support form The support form's "Receiver IBAN" field is a required dropdown filled from GET /bank, which lists only the shared bank accounts. Customers who deposit through a personal IBAN (virtual_iban) can never find their own receiving IBAN there, so today they are forced to pick a shared account they never transferred to - the ticket then carries a wrong IBAN that reads like a statement from the customer. The field is being replaced by free text, and this endpoint is what lets the frontend tell the customer straight away whether the IBAN they typed is one DFX receives money on. PUT /bank/receive-iban takes the IBAN in the body - never in the URL, so it stays out of access logs - and answers with one of four states: DfxIban, InvalidIban, UnknownIban, or LoginRequired. Two filters are deliberately absent. The bank lookup ignores `receive`, because a missing transfer is by nature an old one and a hit on a retired account is still money that reached DFX; filtering would tell a real customer their IBAN does not belong to DFX. The personal-IBAN lookup ignores the lifecycle state for the same reason - an expired or deactivated personal IBAN was still a real receiving IBAN. Personal IBANs are matched only against the requesting account. The guard is optional, so a global lookup would turn this into an unauthenticated oracle over customer-bound IBANs. That is why LoginRequired exists as its own state: without a login the personal IBANs stay unchecked, and answering UnknownIban there would be a false statement to a customer whose IBAN does exist. The endpoint is an input aid only - it enforces nothing, and issue creation still accepts any free text. Comparison runs on the normalised electronic format, since both the stored values and customer input carry arbitrary grouping spaces and casing. BankService takes the VirtualIbanRepository rather than the VirtualIbanService, because that service already depends on BankService and both live in the same module. * fix(bank): correct IBAN normalization and input handling in the receive-IBAN check Review turned up four real defects in the first commit. Normalization stripped only whitespace, so a customer pasting an IBAN with hyphen grouping got InvalidIban for a perfectly valid IBAN. ibantools ships electronicFormatIBAN, which strips both spaces and hyphens and uppercases, so the hand-rolled helper is gone in favour of the library. It also returns null for a non-string, which is now treated like any other unusable input - and it makes the stored side null-safe, where the old regex would have thrown. The DTO no longer carries @Transform(Util.sanitize). That transform runs before @IsString, and Util.sanitizeString calls value.trim() unguarded, so a body such as {"iban": 123} threw a TypeError that the exception filter turned into a 500 - on an endpoint that is reachable without a login. HTML sanitizing is pointless for an IBAN that is structurally validated and normalized anyway. @IsString now rejects a non-string with a clean 400. A comment records why the transform must not come back. UnknownIban was renamed to NotMatched, because the old name asserted more than the check knows. For an authenticated caller the state means "could not attribute this", not "does not belong to DFX": personal IBANs of other accounts are deliberately never checked, and mergeUserData does not move virtual_iban rows to the master, so a customer's own older personal IBAN can land there too. Every state is now documented in the enum, including that NotMatched makes no claim about DFX ownership. The endpoint is reachable unauthenticated, so it now runs behind RateLimitGuard, placed first as in the existing public endpoints. Two tests were added: hyphen-grouped input resolves to DfxIban, and unusable input yields InvalidIban instead of throwing. * fix(bank): accept every whitespace style when normalizing an IBAN Switching to electronicFormatIBAN in the previous commit traded one gap for another. The library strips only ASCII spaces and hyphens; the hand-rolled helper it replaced stripped every kind of whitespace but no hyphens. Measured against ibantools 4.5.1 with a valid IBAN in different groupings, the library alone rejects a non-breaking space, a narrow non-breaking space, a tab and a line break, while accepting ASCII spaces and hyphens. That is not an edge case for this endpoint. An IBAN pasted out of a PDF statement or off a web page very often carries a non-breaking space as its grouping character, and the whole point of the free-text field is that customers paste what they have. Such a customer would have been told their perfectly valid IBAN is invalid. The normalization helper is back, now doing both: it strips all whitespace itself and then hands the value to electronicFormatIBAN, which removes hyphens, uppercases, and returns null for anything unusable. A typeof guard keeps that null-safety for stored values as well. All three comparison sites - the input, bank.iban and virtualIban.iban - run through it. The comment that implied the library normalized completely is corrected. Four cases were added covering non-breaking space, narrow non-breaking space, tab and line break, for a collective account and for a personal IBAN. The separators are written as escape sequences so no invisible character can be lost while editing. The guard was checked by reverting the helper to the library-only form: exactly those four cases fail, and pass again once it is restored. * fix(bank): normalize an IBAN by allow-list and close the untested branches Three safeguards of the receive-IBAN check turned out to be unpinned: inverting them left every test green. The branch order that answers a logged-out customer with DfxIban on a collective account - the single most common path through this endpoint - could be swapped for LoginRequired unnoticed. Checksum validation could be reduced to a shape check, which would answer a customer who mistyped a digit with NotMatched, telling them their IBAN is not ours instead of that it has a typo. And normalization could be dropped from the stored side of either comparison, which matters for personal IBANs because those values are persisted straight from the provider response without validation. One test each now pins these, and each mutation is caught by exactly its own test. Normalization changes approach rather than gaining another character. Stripping whitespace missed the zero-width family (U+200B, U+200D, U+2060, the direction marks) and the soft hyphen, none of which JavaScript counts as whitespace, plus dots, slashes and quotes. That was the third defect in the same function, so the deny-list of separators is replaced by an allow-list of what an IBAN may contain: letters and digits, nothing else. That is complete by construction. An IBAN: prefix stays invalid, which is correct - it is not a separator problem. Three comments overstated what they knew and are corrected. The normalization comment no longer promises to absorb every pasted form. The rate-limit comment no longer promises protection: ThrottlerModule.forRoot() is called without options, so limit is undefined and the guard's comparison is always false - the guard is inert until that is fixed separately, and the comment now only explains the ordering. The DTO comment now names the actual cause of the 500 it avoids, an unguarded value.trim() in Util.sanitizeString, rather than arguing that HTML sanitizing is pointless for an IBAN. Also moved the section header so it no longer encloses the unrelated isBankMatching, and made the constructor uniform by adding the missing readonly. * test(bank): pin the guards, the throttle and the validation boundary Mutation testing found three safeguards that could be removed without a single test noticing. Deleting @UseGuards entirely, or swapping OptionalJwtAuthGuard for a hard AuthGuard, left all tests green. Both are severe: without the optional guard req.user is never populated and every authenticated customer is told LoginRequired, while a hard guard turns anonymous callers away with a 401. The controller spec now asserts the route metadata the way ledger.controller.spec.ts does - path, method, and the guard list including its order - so both failure modes are caught. Removing the DTO validators, or "harmonizing" them with the @Transform(Util.trimAll) that every other IBAN DTO in the project carries, also went unnoticed. The second one is the realistic edit, and it would turn the deliberate 400 back into a 500 on a route reachable without a login. The DTO is now driven through a ValidationPipe built from the exact options in main.ts, asserting a BadRequestException for a number, an array, an object, null, undefined and an empty string, plus a positive case proving a plain string arrives unchanged - which is what catches a transform being added. The throttle is raised from 10 to 60 per minute. RateLimitGuard buckets IPv4 callers by /24, so a whole company network shares one counter, and unlike the one-shot precedents it was copied from - 2FA verification, mail login - an IBAN field gets re-checked while a customer corrects a typo. Ten would have meant two colleagues in one office locking each other out of the form they opened because something already went wrong. Two comments claimed more than the code delivers. The normalization is complete only for ASCII: non-ASCII letters and digits are stripped as separators too, so a label in a non-Latin script can be dropped where an ASCII one is not. That cannot produce a different valid IBAN, but the comment and the test name now say what actually holds. And DfxIban is phrased as belonging rather than as an invitation to pay in, because most matching rows are retired accounts. * refactor(bank): use a camelCase route and correct two overstated comments CONTRIBUTING documents camelCase for URL routes, and the closest sibling for this concept - GET /buy/personalIban - follows it, as do the two read-with-a-body precedents this endpoint was modelled on. The hyphenated route was an unjustified deviation, and renaming it is free right now: the endpoint is unreleased and has no consumer in production. Once the client library ships the call, the same rename would break everyone using it. Only the route literal and the two metadata assertions change; filenames, the enum, the DTO and the method names stay. Two comments claimed more than had been checked. The first said every other IBAN DTO in the project carries @Transform(Util.trimAll); three admin DTOs do not - update-bank-tx and create/update-fiat-output. Narrowed to customer-facing IBAN input DTOs, which is both true and the sharper form of the argument, since those are what someone would align against. The second said the bank table holds grouped IBAN values. Production stores all eighteen rows compact and uppercase; the only grouped value is a test fixture. What actually holds is the invariant behind it: no service writes bank.iban - rows arrive through migrations or by hand - and nothing normalizes the column on write, so a row can carry a grouped value at any time. The test that covers it was right and is unchanged. * docs(bank): argue the missing transform from mechanism, not from a census Three attempts at one comment, each narrowing a claim about what every other IBAN DTO carries, and each still wrong - the last counterexample being create-support-issue, which is customer-facing, login-optional and uses Util.sanitize rather than trimAll. The lesson is not a fourth narrowing. Any claim of the form "every other DTO does X" is either already false or becomes false with the next DTO, and it was never load-bearing: the argument works from mechanism alone. The Util helpers call string methods on the raw value, @Transform runs before @IsString, and a non-string body therefore becomes a TypeError that the exception filter turns into a 500 on a route reachable without a login. That transforms exist on other IBAN fields is enough to explain why adding one here would look like tidying up; how many and which is irrelevant. I swept the remaining comments in the diff for the same shape. Everything else states either this code's own behaviour or a fact measured directly, so no claim now depends on an inventory that can drift. Also names the seed CSV as a third way rows reach the bank table, alongside migrations and manual inserts. * docs(bank): drop every comment claim that depends on an unnamed inventory A comment may describe this code, or name a location a reader can open. It must not quantify over an unnamed set of other files or over production data, because such a claim goes stale invisibly. Three statements still did, and the pattern across the previous rounds was to weaken the quantifier rather than remove the dependency. Removed: that transforms sit on other IBAN fields, which was the fourth variant of the same sentence and carried nothing the mechanism argument had not already established; that no service writes bank.iban, a universal negative over present and future services whose load-bearing half was only ever the column; and that the guard order matches the existing public endpoints. The 60/60 rationale stays, because it names the two endpoints it compares against. The enum lost "most bank rows are retired" for the same reason - it quantifies over the contents of the production table and cannot be checked from the repository. The warning it carried is now grounded in the method instead: the check ignores the receive flag and every lifecycle state, so a long-closed account matches just as well. The merge caveat stays, rewritten to name mergeUserData, since a named function is re-checkable in one grep and the caveat is one of the two reasons NotMatched must not be read as "not a DFX IBAN". The audit also caught the method summary still describing the endpoint in the present tense as reporting an IBAN DFX receives money on, the same framing corrected in the enum a round earlier, and one stale route spelling in a test comment. * docs(bank): drop the last comment clause that quantifies over write paths The previous commit removed one half of this sentence and kept the other, which was the same shape with the verb swapped: "nothing normalizes bank.iban on write" is a universal negative over an unnamed set of write paths, and it goes stale the moment one appears. It was also never the point of the comment - what the test demonstrates is that the comparison normalizes the stored side, which is a statement about this code and needs no inventory at all. * test(bank): type the spec helpers and pin the short-circuit for a logged-in caller Two independent review lanes on the final state turned up eight items. The ordering of the checks was only pinned for an anonymous caller. A logged-in caller whose IBAN matches a collective account must also skip the account-scoped lookup, and nothing asserted that - running the personal lookup eagerly would have passed. That test now asserts it, and inverting the order fails exactly it. The spec helpers carried an untyped mock argument and no return types. The filter shape is now a named local type, so the mock is typed without any. Five comments claimed more than was checked. Two stated frequencies over customer behaviour - that a missing transfer is often old, and that the logged-out collective case is the most common by far. Neither is knowable from here, and both rationales stand without the quantifier. One claimed personal IBANs arrive unvalidated from the provider, which is false for one of the two providers; it now speaks only about this comparison normalizing stored values. One called a fixture a transposed digit where it substitutes one. One asserted that editors convert invisible separators, which is not generally true - the honest reason for escape sequences is that they make the characters visible in review, and that writing them literally went wrong twice here. And the throttle rationale described a consumer re-checking the field in the present tense, for a consumer that does not exist yet. * test(bank): pin the four status strings as the wire contract The enum values are what three repositories agree on: the client library carries its own copy, and the support form derives its wording from them. Every test so far compared enum members, so renaming a value would have passed here and broken at runtime in a customer's browser instead. The literals are now asserted; changing one fails exactly that test. One comment also went out claiming the separator cases had been written literally twice by mistake. That history is not in the repository - the cases arrived escaped and stayed escaped - so for any reader it is unverifiable. The verifiable half of the rationale stands on its own: escape sequences make the characters visible in review and lower the risk of an edit normalizing them away.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 27, 2026 17:45
…e rows (#4412) * fix(custody): do not double a day's portfolio value on duplicate price rows The daily value summed balance x price over every price row of that day, assuming at most one row per asset and day. That assumption does not hold: asset_price.created is a local-time timestamp while the grouping uses UTC, so a snapshot taken shortly after local midnight lands in the previous UTC day. That day then carries two rows for the same asset and its value is reported at double. Measured against a real 455-day price series of one asset: six days carried two rows, and the value chart spiked to twice the correct amount on exactly those six days. Every customer with a Safe sees that chart. The daily value now takes the latest price per asset, decided by timestamp rather than by query order. Days with a single row are unaffected, which the added test pins down alongside the duplicate case. * fix(custody): break ties on equal price timestamps by id Two price rows with the same created timestamp kept whichever came first in the list, and the query orders only by created - so identical data could produce different chart values. The higher id is the later insert and now wins, decided from the data rather than the list order.
) * feat(custody): let an owner's own grant narrow their access level Until now the owner of a custody account always received WRITE, and the authorisation actually granted on that account was never consulted. That made one arrangement impossible to express: an authorisation in which the owner keeps inspection only and reserves acting for someone else. An active grant an owner holds on their own account now decides their level. Without such a grant nothing changes — the owner keeps full disposal, which is every account in production today. Managing grants stays tied to ownership rather than to the level, so an owner who narrows themselves can still hand the mandate back at any time and cannot lock themselves out. Reading is unaffected as well: a narrowed grant withdraws acting, not sight, and the holdings are the owner's either way. Adds the first test suite for CustodyAccountService, covering both paths and in particular that an inactive grant never narrows anything — deactivated history must not take part in authorisation. * feat(custody): enforce the access level where acting actually happens Two review findings, both real. First, the narrowing was unreachable. Self-grants are refused and the owner's grant row could not be modified, so no API path led to the state the previous commit reacts to. An owner may now re-level their own grant — limiting themselves to inspection and taking the mandate back. Revoking that row stays refused: it would leave the account without an owner row and make the level unrecordable. Second, and worse: order creation never consulted the access level at all. It runs under the custody role and reaches the Safe on its own, so an owner limited to inspection could still trade. Hiding buttons in the frontend would have been decoration — the API accepted the order regardless. createOrder and confirmOrder now refuse when an own account is limited to inspection. Orders address a whole Safe rather than a single account, since balances and orders carry no account today. Any own account narrowed to READ therefore blocks acting: the order could touch exactly those holdings. Fail closed rather than guess which account an order belongs to. Verified against a running instance in both directions: with the narrowing in place the order is refused with 403, and with the mandate restored the very same request succeeds. Accounts without a narrowing grant — every account in production today — are unaffected. * fix(custody): keep a narrowing in force on a blocked account, cover the order paths Two review findings. The acting check only looked at active accounts. Blocking or closing an account would therefore have lifted the restriction — exactly when caution matters most. Everywhere else a non-active account counts as absent and grants nothing; here absence would have granted something, namely the right to act. The status filter is gone and the test now insists a blocked account stays restricted. Not exploitable today, since no code path ever sets that status, but it would have been a trap for the first account-blocking feature. CustodyOrderService had no test suite either, so the two new call sites rested on manual verification alone. It now has one: both paths refuse when acting is narrowed, both pass the right identity rather than one of the two JWT ids, and a stranger is turned away on ownership before the narrowing check runs — so nobody can learn from the response whether an account is restricted. Also documents why the gap between check and write is left unlocked: only the owner manages grants and only the owner narrows themselves, so the sole party who could win that race is the one who may lift the restriction outright. * fix(custody): keep grant management reachable on a blocked account A narrowing blocks the owner's whole Safe and deliberately ignores account status. Grant management, however, went through requireOwner, which demanded an active account. Blocking a single account would therefore have stranded the grant on it: the owner could neither lift a narrowing they had placed there nor withdraw a stranger's access, and since one narrowing blocks every account they hold, a single block would have frozen the whole Safe with no way back. Grant management now depends on ownership alone. Blocking an account governs what may be done with it, not who decides that. Missing and foreign accounts still yield the same Forbidden, so existence stays unprobeable. Also corrects a comment that still described the status filter removed in the previous commit, and sorts an import. * docs(custody): correct the docstring on getCustodyAccountById It still claimed to be shared by checkAccess and requireOwner. Since the last commit requireOwner resolves the account itself, without the status filter, so that blocking an account cannot strand the grants on it. Only the data path goes through here now. * fix(custody): refuse to issue new grants on an account that is not active Making grant management independent of account status went one step too far. It was meant to keep an owner from being stranded — able to lift a narrowing they placed on a blocked account, or withdraw a stranger's access. Issuing a new grant is neither. Widening the circle of authorised people during a hold is exactly what a hold is meant to prevent, and it is no way out of one. grantAccess now refuses on a non-active account, and does so before resolving the address, so the response cannot reveal whether a mail address is registered. Withdrawing and re-levelling stay open. grantAccess and getAccessList had no tests at all; both are covered now, including that an owner can still inspect grants on a blocked account. The blocked-account test for updateAccess used an unconditional stub that would have stayed green if a status filter crept back into requireOwner — it now evaluates the where clause, verified by reintroducing the filter and watching the test turn red. * fix(custody): refuse to raise a stranger's level while an account is held Refusing new grants on a held account left the same door open one step over: raising an existing grant from inspection to acting widens someone's authority just as much, only through an existing row instead of a new one. It takes effect the moment the account is released, without anyone looking again. The rule is now uniform for a held account: rights may be taken away, not handed out. Lowering a stranger stays open, as does anything on the owner's own row, which is their way out of a narrowing. Uses BadRequestException rather than ConflictException, matching six existing places that refuse an action on an inactive or blocked resource; Conflict is reserved for duplicates. * chore(custody): tidy up after the review rounds Removes an import left unused when the exception type changed, uses the nullish operator the repo prefers, and records two assumptions that were only in my head: elevation is recognised by comparing the two levels that exist, so a third would have to turn it into an ordering comparison; and the status is read before the write, so whatever introduces a hold must deactivate that account's grants in the same change. Also covers granting write on a held account, not just read — the refusal must not depend on the level asked for.
* Treat an unobserved order outcome as unknown instead of failed
A liquidity order whose request left our side without an observed answer was recorded as
Failed. That asserts knowledge we do not have, and it is not inert: a failed pipeline pauses
its rule, and the rule auto-reactivates after reactivationTime, so the same request is issued
again a few minutes later.
Two Scrypt withdrawals hit exactly this path and were mis-recorded. Both requests timed out
waiting for the BalanceTransaction update; both were stored as Failed with no correlationId,
and both had in fact executed at the venue - the exchange transaction and the balance drop
match the order amount to the cent. No double withdrawal resulted, but only because the
balance had already fallen by the time the rule retried. That balance is served from a push
cache with no freshness guarantee, so the safeguard cannot be relied on in the very situation
that produces the timeout.
Four changes, following the payout subdomain, which already solves this problem for
blockchain broadcasts:
1. Reserve the venue reference before sending. Scrypt is the one integration that lets us
choose it (ClOrdID / ClReqID), but it was generated inside the service and only returned on
success, so a timeout lost it for good and left nothing to look up. Integrations may now
supply a reference through reserveCorrelationId, which the pipeline persists before the
request goes out; the id is derived from the order id, satisfying the venue's daily
uniqueness requirement without a random component.
2. Add LiquidityManagementOrderStatus.UNCERTAIN, the counterpart to PAYOUT_UNCERTAIN. It is
terminal for the pipeline - checkRunningPipelines already leaves such an order alone, so no
rule is paused and nothing auto-reactivates - but not for the order.
3. Classify the send boundary fail-closed. Request timeouts now carry their own error type
rather than a message string, so they can be told apart from a dropped socket, which proves
the request never completed. Only the latter stays an ordinary failure. As a consequence
idempotent reads - fetch and fetchAll, which cannot affect venue state - retry on timeout
instead of ending the whole order; that alone covers 47 of the 49 timeouts observed over
two weeks.
4. Reconcile instead of repeat. resolveUncertainOrders asks the venue what happened and never
re-sends. Absence only counts as proof after a grace window, an unreachable venue leaves the
order in quarantine, and reconciliation runs before any new order is issued.
Observability, so a quarantined order is not silently parked: uncertainLmOrderCount is exposed
on the liquidity observer, the quarantine mail is pinned per order and debounced, and the
pipeline failure mail is pinned per rule and debounced - one incident used to send a mail per
retry. The success mail is dropped; it accounted for 211 of 255 liquidity mails in a week and
carried no information, which is what made the mails that matter unreadable.
Also fixes an unrelated spin: an error outside the three known exception types left the order
in Created, and startNewOrders reported a change regardless, so the caller's while loop could
not terminate.
* Close the remaining unconfirmed-outcome paths found in review
Three gaps in the first pass, all of the same shape as the bug it fixes.
The amend boundary was still open. checkTrade may cancel-replace or restart an order from
inside the completion check, and both created a fresh reference that was never recorded, so a
replacement whose confirmation never arrived could not be looked up even in principle - and the
error fell through to OrderFailedException, which pauses the rule and reissues the trade.
Callers now supply the replacement reference, derived from the order row as
<prefix><orderId>-<referencesUsed>, so it is reproducible without an extra column. An
unconfirmed outcome there quarantines the order, and reconciliation enumerates the
replacement candidates rather than only the current reference.
"Connection closed" was treated as proof that nothing was sent. It is not: requestWithId hands
the payload to the socket before registering the pending entry, and a later close rejects that
entry with a generic message that says nothing about whether the venue acted. The
classification is now fail-closed - only an explicit rejection from the venue, which proves the
request was seen and refused, keeps an error an ordinary failure; everything else is an unknown
outcome. Over-classifying is self-correcting, because an error that truly happened before the
send leaves no trace at the venue and reconciliation settles the order as failed after the
grace window.
hasPendingOrders did not count quarantined orders. That gate is what stops a second rule on the
same exchange from acting while funds are unaccounted for, so an unresolved order has to block
there. Same reasoning applied to getProcessingOrders, and to getPendingTx, where omitting the
status would have dropped the amount in question out of the financial log.
Alert debouncing was keyed by rule alone. Suppression compares only the key, never the body, so
a genuinely different failure of the same rule was swallowed within the window. The key now
includes a digit-insensitive hash of the cause: true repeats still collapse, a new cause still
reports. Keying by pipeline instead would have restored the flood, since every retry is a new
pipeline.
Tests: the amend and reconciliation paths, quarantine on socket close, a venue rejection staying
an ordinary failure, replacement-reference derivation, a pipeline left untouched while its last
order is quarantined, and the timeout type and read retry in the connection itself.
* Make the amend boundary actually reachable, and widen the pending checks
Follow-up to the previous commit, from a second review pass.
The amend fix did not fire. checkTrade's inner catch swallowed every error that was not
TradeChangedException, attempted a best-effort cancel and returned false, so a timeout or a
dropped socket during editOrder never reached the classification added for it - the order kept
polling the old reference while a replacement created under the reserved one stayed invisible,
and reconciliation never ran because the order never entered quarantine. Unless the venue
explicitly rejected the amend, that path now raises a dedicated unconfirmed-write error, and the
same guard wraps the restart in the cancelled branch.
That error type is what the check path was missing. Message matching cannot separate a
dropped socket on a read from one on a write, yet the first is harmless and the second is
unresolved; the completion check now tests for the write boundary before the transient-error
branch, which previously classified a connection drop during a restart as retry-next-tick. The
rejection markers move next to the error types so the send path and the check path share one
definition, extended with the edit rejection.
The custom-asset balance sum was left out of the previous pass on the grounds that Scrypt is
not among its systems. That reasoning was wrong: the generic quarantine in startNewOrders is
not system-specific, so a Kraken or Binance order can reach the status too, and neither adapter
can resolve it - its amount would silently drop out of the balance for as long as it stayed
there.
Tests cover the amend boundary end to end: an unconfirmed write surfaces as an unknown outcome,
carries the replacement reference into the quarantine reason, and a plain dropped connection on
the read path still resolves as retry-next-tick.
The PR description has been rewritten to the final state; it still described the first
iteration and, on the dropped-socket classification, asserted the opposite of what the code now
does.
* Cover the amend write boundary at the level where it used to be swallowed
The previous commit made an unconfirmed amend propagate out of checkTrade, but the guard was
only exercised through the adapter. The defect lived one layer below, in the service, and the
service had no checkTrade tests at all - which is why the first attempt at this fix looked
right and did nothing.
Three tests at that level: an unconfirmed amend leaves checkTrade as an unconfirmed-write error
instead of being cancelled away and reported as "not complete"; the raised error carries the
reserved replacement reference, so the order can be reconciled against it; and an amend the
venue explicitly rejected still falls back to cancel-and-continue, since a rejection proves
nothing was created.
* Stop inferring outcomes that were never observed, in four more places
A cross-vendor review of the previous state found four more spots where the code still concluded
something it had not seen. Each is the same mistake the PR set out to remove.
A CREATED order that already carries a reserved reference is no longer re-sent. That
combination can only arise when a previous pass reached the send boundary and died before
recording the result, so sending again is precisely the duplicate we are trying to avoid; it now
goes straight into quarantine.
The catch-all no longer quarantines everything. Whether a reference was reserved tells us
whether the send boundary was crossed at all: without one, nothing can have been transmitted and
an ordinary failure is correct. Quarantining those stranded configuration and factory errors in
a state only a human can clear - and, since reconciliation needs the same integration that just
failed to load, they could never clear themselves.
Reconciliation checked the oldest reference first. A replaced order usually still exists at the
venue in a cancelled state, so the original matched, the order was reported as sent, and the
live replacement stayed untracked while the completion check polled a dead reference. Candidates
are now tried newest first.
Absence is no longer treated as proof of non-arrival. The venue offers no terminal "this
reference was never accepted" reply, so a missing record after any amount of time is not
evidence; concluding otherwise is what would let a rule reissue a request that later
materialises. Such an order stays quarantined for a human, and its rule stays blocked. The grace
window existed only to make that inference safer and is gone with it.
Two more, from the same principle. A failure to READ an order the venue has acknowledged no
longer fails it - that would release the rule to open a second position against the same funds -
but is retried, with a dedicated type for "acknowledged, then vanished" that quarantines instead
of failing. And the quarantined status is taken back out of the financial log's pending set: the
log adds a pending amount back to the balance and nets it against the venue's locked funds, but
an unsent order locks nothing, so counting it would inflate equity by its full amount - the one
error direction that can hide a real loss from the safety threshold.
* Give a venue rejection its own type, and a quarantined order a way out
A follow-up review found four more places where the code still had to guess.
Rejections were recognised by message text, and one real terminal path phrased its message
differently ("Order <id> has been rejected"). It matched nothing, so after the previous commit
made unmatched errors retry instead of fail, a genuinely rejected order would have been retried
forever. Rejections now carry their own type, used by every path that turns a venue refusal into
an exception: it cannot be missed by rephrasing, and a transport error that happens to quote the
phrase can no longer masquerade as a settled outcome.
Reconciliation recovered a withdrawal from the venue's history but never cached it, while
getWithdrawalStatus reads only the cache. An order leaving quarantine on the strength of that
lookup would have polled a reference the cache still did not know, and never completed. The
match is now fed back through the same terminal-aware cache write as a live push.
A bare request timeout in the completion check can only come from a read, because every write
there is already wrapped. Quarantining it stranded an order that nothing at the venue had
touched, so it is retried like any other read problem.
And the gap left by the previous commit: with absence no longer proving non-arrival, nothing
released a genuinely unsent order, so its rule would stay blocked indefinitely with no operator
action available. There is now a guarded admin endpoint, modelled on the payout subdomain's
retry guard - the caller must assert that the venue was checked and name where, and the
assertion is recorded on the order.
Also corrected: a comment claimed the socket send precedes registering the pending request,
where the code does the reverse; the conclusion it supported is unaffected, since what makes a
close ambiguous is that the bytes may already be on the wire. Stale grace-window wording removed
from code and description, and the description now matches the final behaviour, including the
deliberate exclusion of the quarantined status from the financial log's pending set.
* Settle the last paths where a refused write could still repeat
Seventh pass. Six findings taken, one declined.
A refused amend now reports itself. checkTrade swallowed the refusal and returned "not
complete", so the caller never learned that the replacement reference was spent - the venue
requires them to be unique, so the next tick derived the very same one and the pair could loop.
The refusal carries the spent reference, the order records it, and the derivation moves on while
tracking stays on the original, which a refused amend leaves live.
Reconciliation no longer adopts a rejected replacement. Any returned record counted as proof of
sending, so a replacement the venue had refused would be adopted, immediately fail the order,
and release the rule although the original was still standing.
Both resolution paths now write conditionally. Automatic reconciliation and an operator can hold
the same order at once, and an unconditional save let whoever finished last win - including
failing an order the venue had just confirmed as live. The status is part of the WHERE clause
and a lost race is skipped, or reported as a conflict on the manual path.
A venue error reply is no longer a plain error. It carries its own type, deliberately distinct
from a rejection: "unknown reqid" arrives the same way and means the venue lost our request
context, which for a mutation is as open as silence.
A cached withdrawal record that is not terminal no longer short-circuits the history lookup -
the missing terminal push is exactly what such a record would be hiding.
The manual endpoint was too thin: the asserted flag and the caller were dropped at the edge, a
whitespace-only reference passed validation, and the log line preceded the write. The service
now re-asserts the claim, normalises and caps the reference, records who authorised it, persists
before logging, and returns nothing rather than the entity.
DECLINED: extending this to the ccxt venues. They share the gap, but they have no client order
id today and no reconciliation lookup, so propagating the unknown outcome would quarantine
orders across every exchange with no exit but a human - a worse position than today, on venues
where nothing has gone wrong. Recorded in the description as its own piece of work.
* Bound the audit reference and record the consumer impact
Conformity pass. The manual-resolution reference was unbounded while both analogous DTOs in the
repository cap it at 1024 characters, and it was written to the log untruncated; the cap now
lives in the DTO, so the service records the validated value instead of silently shortening it.
Two update mocks in the tests dropped their types for no reason and are typed again.
On the contract question: the new order status appears in no DTO, and both endpoints that expose
these orders are admin-guarded and excluded from the published surface, so there is no public
contract change. Whether the internal admin view enumerates statuses exhaustively cannot be
determined from this repository, so the description asks the reviewer to confirm it rather than
leaving the question unasked.
* Let an observation outrank a judgement when both resolve the same order
The compare-and-set added earlier decides who writes first, and first is not the same as right.
If an operator releases a quarantined order in the same moment reconciliation confirms the
request exists at the venue, the operator's write lands, the order is marked failed, the
pipeline releases and the rule can issue a second order against a live position.
The manual path now asks the venue itself before releasing, and refuses when the reference is
found - a positive observation outranks the operator's judgement, and the order is left for the
next reconciliation pass to move on. A lookup that cannot be performed does not block the
release: the operator has asserted an independent check, and a reference the venue can no longer
be asked about must not turn into an order nobody can ever clear.
* Make a replacement reference durable before it is sent, and bound the unobservable case
Three findings from the sixth pass, all of them consequences of the previous round's fixes.
A replacement reference was only recorded once it was accepted or explicitly refused. In between
it was neither the current reference nor a spent one, so a replacement whose confirmation was
lost could be derived a second time. The reference is now claimed and persisted immediately
before the request goes out, for both the amend and the restart.
Reconciliation could still fall through to a superseded predecessor. An accepted replacement may
simply not be visible at the venue yet, while the order it replaced still is; matching the older
one reported the request as sent and left the live replacement untracked. An older reference is
now considered only after the newer one was explicitly rejected - anything else keeps the order
quarantined.
A positive observation on the manual path was refused but not recorded. The row stayed
quarantined, so a later attempt made while the venue happened to be unreachable could still
release it and undo what had been seen. The observation is now persisted first, which takes the
order out of manual reach entirely.
And an acknowledged order that cannot be observed no longer polls forever. It was kept
deliberately - failing it would release the rule against a live position - but the manual path
only accepts quarantined orders, so there was no way out at all. Past the same age at which the
venue itself is considered to have lost an order, it is quarantined: still not declared failed,
but now reachable for a human.
* Reconcile only references that were really sent
The seventh pass found that the previous commit had broken the very case this work exists for.
Reconciliation built its candidate list by deriving the NEXT reference and checking that first.
For a freshly quarantined order that reference has never been sent, so it was absent — and since
the previous commit made an absent newest reference stop the search, the reference that had
actually gone out was never looked at. Every quarantined order would have stayed quarantined
until somebody released it by hand. The tests hid it by answering every queried reference.
Reconciliation now walks exactly the references this order has put on the wire, newest first,
ordered by attempt number rather than by how the list happened to be stored. Nothing is
synthesised.
Two more from the same pass. The age bound that stops an unobservable order from polling forever
sat behind the transient-transport branch, so an old order whose socket kept dropping never
reached it — a dropped socket that keeps dropping is, from here, indistinguishable from one that
will never answer, so the bound now applies to both.
And a positive observation could still lose the write race: if somebody released the order as
not executed while reconciliation was watching the venue confirm it, the release landed and the
observation was discarded. An observation outranks a judgement, so it is now taken back — the
order returns to in progress, with an alert, because whoever released it needs to know their
check missed something.
* Stop three silent waits, and keep the reclaim from overruling the venue
Five findings from the eighth pass.
The reclaim introduced last time was too broad. It matched any failed row by id, so an order that
had ended for an entirely unrelated reason could be resurrected by a late positive observation.
A not-sent resolution now stamps its failures, and only a failure carrying that stamp can be
taken back — the reclaim exists to overrule a judgement, never the venue.
The manual path ignored whether its own positive observation actually landed. If a negative
resolution won the write, the observation was discarded; it now reclaims in exactly the same way
as automatic reconciliation.
Three paths could wait forever. An acknowledged withdrawal whose terminal update is never seen,
an order left in a pending status whose update is missed, and - already bounded for trades - the
unobservable case, all answered "not complete" indefinitely while the manual path accepts only
quarantined orders, so there was no way out at all. All three now fall under the same age bound
and become unknown outcomes rather than failures.
And reconciliation can conclude a negative again. When every reference an order ever put on the
wire comes back rejected, nothing was created — unlike mere absence that is a real negative, so
the order is resolved instead of being asked about forever while its rule stays blocked.
* Fold the pending-order age bound into the branch that already existed
The previous commit added a second case block for the pending statuses instead of extending the
one already there, which is a duplicate-case lint error. The bound now lives in the existing
branch, and the sixty minutes it shares with the "order cannot be found" path is a named
constant rather than a literal repeated in two places.
This should have been caught before pushing: the lint run had already reported it and the commit
went out anyway.
* Tell an observed wait apart from an unknown outcome
The ninth pass found the two drifting together again. The previous commit quarantined an order
that had been pending too long, and a withdrawal the venue knew about but had not settled. Both
are OBSERVATIONS - we know exactly where the order stands - so quarantine was the wrong shelf:
reconciliation found the reference, handed the order straight back as sent, and the next
completion check quarantined it again. Every transition reported a change, so the pipeline loop
kept going round, querying the venue each time, while manual release refused because the
reference existed.
Quarantine now means only what it says: we cannot tell whether the request took effect. A
pending order simply waits, however old it is, and a withdrawal the venue has a record for waits
too. Only the genuine blind spot - no record at all, past the age at which the venue is
considered to have lost an order - is still quarantined. An order that waits too long is a stuck
order, which the monitoring counter already surfaces; it is not an unresolved one.
The same pass found no route to an automatic duplicate execution across the initial send, amend,
restart, rejection, reference-claim and reclaim paths.
* Recover a lost adoption before writing again, and quarantine only a real blind spot
Two findings from the tenth pass, plus three conformity points.
A replacement the venue had accepted could be lost. The claim persists the new reference before
sending, but adopting it afterwards is an ordinary save - and if that save failed, the row still
named the predecessor while the venue had already cancelled it. The next check restarted from
that predecessor and placed a second order alongside the live replacement. Before anything may
write again, a claimed reference the venue is working is now adopted; a rejected one is skipped.
The age bound was too broad. It fired for any failure after the check began, so an order the
venue could still show us was quarantined because pricing or aggregation had failed - and
reconciliation, finding the reference, handed it straight back for the next check to quarantine
again. The bound now applies only when the order itself cannot be observed.
From the conformity pass: the route follows the repository's camelCase majority (70 to 32); the
audit reference is trimmed before validation, as eleven other string DTOs do; and the new tests
use typed venue fixtures instead of widened literals.
Not changed, with reason: the demand to remove every `any` from tests contradicts the verified
house convention - the pattern appears 1441 times across this repository's specs. The cases where
typing was straightforward are typed; the convention is not overturned in passing.
* Adopt only forwards, and never write past a claim nobody can account for
Three sequences found in review could still end in a second request against the same funds.
A predecessor is not a replacement. After an amend this row DID record, the superseded
original sits in the reference list as cancelled — and adoption, which only skipped the
current reference, would take it back and restart the very quantity the replacement was
working. Adoption now considers references newer than the current one only.
A claimed reference the venue does not show, or cannot be asked about, is not a refusal.
It is recorded before the request leaves, so it may be live at the venue right now; the
check may not step past it and carry on with the predecessor. Such a claim now blocks
every write on the order, and the wait ends the same way any other blind spot does: past
the age at which the venue itself is considered to have lost an order, it goes to a human.
Applying a positive observation takes two writes — release the quarantine, or take back a
negative resolution that got there first. A crash or a failed write in between left the
order in a state nothing ever read again, while its rule was free to plan anew. Failures
stamped as not-sent are now re-examined alongside the quarantined ones for an hour, so the
second write is simply retried.
Six of the seven added tests fail without the corresponding change; the seventh pins the
existing two-step reclaim.
* Let a recorded not-sent failure stay reclaimable, and use PUT for the release
An observation that the venue does know the order becomes durable in exactly one write. If
that write does not land — the process ends, the statement fails — the observation is gone,
and the order sits failed while the venue is working it. The hour-long window that covered
this was itself a way to lose it, so there is no window any more: a failure stamped as
not-sent stays eligible for reconciliation without an age limit.
What that would otherwise cost is a venue lookup per settled failure on every tick, for the
rest of its life. So each such order is asked about once per process instead. That is the
same coverage — the observation can only be lost by this process ending or its write
failing, and neither survives into the next one — without the standing load.
The release endpoint updates an order, so it is a PUT, per the REST section of
CONTRIBUTING. One test also went back to the typed venue-order helper instead of casting a
literal.
* Say it out loud when a confirmed observation cannot be written back
Two review passes chased the same thing from opposite sides: re-examining terminal
failures kept a lost observation recoverable, but it also meant a leading-wildcard scan
over every historical failed row on each pipeline run, and the memo that bounded that cost
treated an inconclusive lookup as settled — so a delayed confirmation was suppressed for
the life of the process.
Both go away with the mechanism. Reconciliation is back to quarantined orders only. The
reclaim stays: it is one conditional statement and it still catches a negative resolution
that lands mid-pass.
What was missing was never the scan, it was the admission of failure. When the venue has
confirmed an order and neither release matches — or the write throws — that observation is
the one fact worth having, and it was being dropped as a log line. It now raises an alert
naming the order and the reference, telling whoever reads it to treat the order as live at
the venue. Consistent with the rest of this change: where we cannot be sure, a person
decides, and no rule plans against those funds in the meantime.
A release someone else performed correctly is recognised and stays quiet.
* Hold a confirmed order blocking, rather than only reporting it
An alert is read at human speed; a rule reactivates in minutes. So reporting that the
venue confirms an order — while the row stays failed — still leaves the scheduler free to
plan against funds that are already committed. Reporting was the wrong half of the answer.
Anything short of a clean release now puts the order back into quarantine, which is the
state this subdomain already treats as in flight with an open outcome: no rule plans
against it, and the next reconciliation pass simply tries again. The alert stays, but on
top of a state that holds by itself.
The manual release had the same shape and now takes the same path: its refusal no longer
depends on a write it never checked. Its message says which of the two happened.
The report also no longer depends on a second read succeeding — that read is only what
lets a genuinely safe outcome stay quiet, and a state that cannot be read is not one of
them.
* Bound the second look by observability, and stop overwriting what a row already says
A not-sent failure is eligible for reconciliation again, so an observation whose writes
never landed — including the re-quarantine that normally catches that — is still applied
afterwards. The bound on it is the venue's own reach: Scrypt's execution history goes back
thirty days, and past that nobody can make an observation to apply. That is deliberately
not a deadline for applying one, which is how the previous attempt could lose it; it is
also what keeps the query off the entire failure history.
Second: overruling a resolution was erasing the record of it. Both write-backs built their
message from the copy of the row this pass started with, which predates the resolution
being overruled — so a reclaim replaced the account that released the order and the
reference they checked with a stale string. Both now read the row and append to what it
actually says, and where it cannot be read, only the status changes.
* Mark a not-sent resolution on the row instead of hunting for it later
Finding the failures that still owe reconciliation a second look was done with a wildcard
match over every failure ever recorded, on a job that runs every ten seconds, and the
attempts to bound that cost kept trading one hole for another: an in-memory memo lost the
obligation on restart, a time window turned into a deadline for applying an observation.
The obligation now lives on the row. A not-sent resolution stamps notSentResolvedAt; the
first reconciliation pass that looks at the order again clears it. That is the coverage
that was wanted — the pass writing such a resolution may be racing one that has just
watched the venue confirm the same order, and an observation that could not be written
would otherwise be gone — and it survives a restart, because it is a column and not a set
in memory. Cleared after that one look, so settled failures are not put to the venue every
ten seconds for good. Indexed, so finding them is an equality predicate.
Two smaller things, both about not destroying evidence:
The reclaim and the re-quarantine now guard their write on the exact reason they just
read. Another resolution can land in between, and appending to the older copy would erase
the newer operator and reference.
And the verification reference is trimmed at the edges only. Util.trimAll removes every
space — it is for identifiers like IBANs — and it was turning "venue console, ticket
OPS-42" into one run of characters. That field is evidence somebody has to read.
The migration is additive and nullable, with no backfill. It was not run against a
Postgres instance: this machine has none.
* Guard the recheck marker against a newer resolution, and keep the moment in the reason
Clearing the marker was guarded by order and status alone, so a pass that had been
looking at one resolution could clear the marker of a newer one written in the meantime —
dropping the further look that newer resolution was owed, which is the whole point of the
marker. The clear is now conditional on the exact marker the pass started from.
A marked order whose integration has no lookup at all could never have its recheck
happen, so the marker stood forever and every pass selected the row only to skip it. Such
a row is released instead.
And the column is named for what it is: work still owed, not a record of when the
resolution happened. That moment now goes into the order's own reason, which nothing
clears — so releasing the marker cannot make it unrecoverable.
* Remove the superseded migration file left behind by the rename
The rename to notSentRecheckDue added the new migration but left the old one in place, so
both would have run and the table would have gained two columns instead of one. Only the
new file belongs here; neither has been merged, so there is nothing deployed to correct.
* Tell an unanswered lookup apart from an unanswerable one
A venue that cannot be reached was reported the same way as a venue that answered and had
no record. Both arrived as UNRESOLVED, so the recheck an order still owed was retired on
the strength of a lookup that never happened — and with it the observation it was being
kept for. A failed lookup is now UNAVAILABLE: no question reached the venue, so nothing
was looked at, and the order stays marked until something actually is.
Separately, an order can outlive the adapter that made it. The integration factory returns
null for a system or command that is no longer registered, and both the reconciliation
loop and the manual release dereferenced it: the loop would have thrown on every pass
forever, and the manual release refused an order that nothing could ever look up again —
leaving it quarantined with no way out. Both handle the absence now, and a marked failure
nobody can ask about is released rather than selected and skipped for good.
* Make a not-sent release take effect only once the venue has answered
A release said the request never left, and the order became a plain failure on the spot.
That is terminal: the pipeline finishes, the rule reactivates, and it may issue the request
again — all while a confirmation that the order IS live could still have been in flight and
simply lost its race to the write. Keeping such an order reconcilable afterwards did not
help, because between the release and any later correction the rule was already free.
So the release no longer ends the order. It is recorded, the order stays quarantined, and
reconciliation puts it into effect on the next pass — normally seconds later — once the
venue has answered that it has no record either. Two negatives, one of them from a person
who looked. A venue that cannot be reached answers nothing, so nothing happens and the
order keeps blocking; a venue that confirms the order overrules the release outright.
An all-references-refused verdict still fails the order directly. That is the venue's own
answer, not a judgement, and it needs no confirming.
This removes the reclaim path entirely: nothing writes a failure behind a pending release
any more, so there is no negative resolution left to take back.
* End an order only against the release that was actually examined
Ending an order is the one step here that cannot be taken back, so it now compares and
sets on the exact pending release the pass looked at. A release written since has a
confirmation of its own outstanding, and completing the older reading would skip it.
The one case where a release does not wait for the venue — an order no integration can
look up any more, where no answer can ever come — is now stated in the same places the
rule is: the column, the DTO, the migration and the pull request text. It was only in the
code.
The release tests asserted against the in-memory entity, which the reconciliation loop
would have left looking correct even if nothing had been stored. They now assert the
conditional write itself, its predicate and its payload, and that the pass reports a
change.
One comment still described the reclaim path that was removed.
* Retry an observation the database refused, instead of dropping it
When the venue confirms an order and the repair that holds it blocking cannot be written,
the failure was swallowed. The order stayed a plain failure — nothing selects one of those
again — while the venue worked it, and after the rule reactivated the request could go out
a second time.
The write is kept and repeated at the start of each pass, ahead of any new lookup. It asks
the venue nothing; it only repeats what is already known, and it stops as soon as it lands
or another path has put the order somewhere safe. Held in memory deliberately: the
alternative is a durable queue nothing ever drains, and the one case this cannot cover —
the process ending first — is covered by the alert that goes out at the same moment, which
names the order and tells whoever reads it to treat it as live.
Also: the branch that completes a release for an order nothing can look up now reports the
change like every other branch, so the caller's loop sees it.
* Make a confirmed order safe with one narrow write, and stop the pass until it is
Two ways a confirmed venue order could still end up terminal.
The first: applying an observation is a substantial write, and everything about it can
fail. So the very first thing now is the narrowest write in the file — one column on a row
that is still quarantined, cancelling any pending release. Once that lands, no judgement
can end the order, whatever fails afterwards and even if this process stops. It is also
what makes a quarantined row genuinely safe to stop retrying on: quarantined WITH a release
pending is one inconclusive lookup away from being ended, and that is exactly what the
observation contradicts.
The second: holding an unwritten observation in memory kept it alive, but the rest of the
pass carried on regardless — starting pipelines, advancing them, issuing orders — on a
picture in which an order says failed while the venue is working it. The pass now stops
after reconciliation for as long as any observation is unrecorded, and the next one retries
the write before asking the venue anything.
* Repair a confirmed order with the first write, and do not trust a cache after an unconfirmed cancel
The first write for a confirmed order only stripped a pending release of its power. If a
release had already ended the order, that write matched nothing and the repair came later
— so until it did, the only thing between a live venue order and a second request was this
process staying alive. The same statement now also puts an ended order back into
quarantine. Two columns, no appended text, nothing it depends on; the reason why still
follows separately, and if that never lands the order is at least still blocking.
Second: when an amend is refused, the fallback cancel is a write as well. Its failure was
noted and dropped. Unconfirmed, that cancel may have taken effect at the venue while the
cached report still shows the order open — and a non-terminal cached report is never
replaced by a later fetch, so every check afterwards would keep waiting on a picture that
cannot change, with the order stuck in progress and out of reach of both reconciliation and
the manual path. The cached report is dropped instead, so the next lookup has to ask the
venue, and the refusal says the cancel went unconfirmed.
* Do not let an unreachable venue veto a verified release forever
A release waits for the venue to answer, which is right — but a venue that answers nothing
at all would have held a verified order for good. The wait exists to catch a confirmation
that is in flight right now; after an hour of silence there is none in flight, only an
operator who checked and is being ignored.
This is a liveness bound, not a safety one, and nothing is concluded from the silence: the
person who released the order concluded it. Silence merely stops being a veto.
Also: an import out of alphabetical order.
* Put the release rule where it belongs, and state both its exceptions everywhere
Whether a pending release has waited out an unreachable venue is something the order
knows about itself, so it is answered by the order, with the bound alongside it, and the
service is left with the orchestration.
And the promise that a release waits for the venue was written in six places, all of which
still named only one way out of it. Both are now stated wherever the rule is: an order no
integration can look up, and a venue that has answered nothing for long enough. Neither
concludes anything from silence — silence stops being a veto on the person who checked, it
never becomes evidence.
The entity gains its own tests, including the boundary either side of the wait.
* State both release exceptions in the DTO and the migration too
The same promise is written in several places and two of them still named only one way
the wait can end. No behaviour change.
* Sort the entity imports by path
The new import went in at the top instead of in order, and the two around it were already
out of order. No behaviour change.
TaprootFreak
approved these changes
Jul 28, 2026
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist