Release: develop -> main - #4399
Merged
Merged
Conversation
The structured debug endpoint could show that an amlCheck transition happened and which code path caused it, but never who approved it: amlResponsible was blocked by the startup invariant that forbids any overlap with the /gs/db masking list. That left "who released this transaction" unanswerable from the audit trail. Allowlist the column and introduce DebugRestrictedOverlapExceptions, an explicit registry of (table, column) pairs that may overlap. Every unregistered overlap still aborts module load, and a counter-check rejects stale entries so the list cannot rot into a silent gap. /gs/db masking is untouched, and `comment` on the same table stays blocked. Extract the invariant into the pure, exported assertDebugAllowlistInvariants so the guard itself is testable: asserting the real constants only proves today's data is consistent, not that the exception matches per column rather than per table. Covered by synthetic fixtures, including the case where excepting one column must not amnesty a second overlap in the same table.
* fix(dev): make the documented local quick start work The quick start in the README (cp .env.local.example .env, docker compose up -d, npm run setup) currently fails on a clean checkout. Two independent causes: 1. The config requires REALUNIT_W2W_GAS_LOW_BALANCE_THRESHOLD and throws when it is unset, but the variable is missing from .env.local.example and is not generated by the setup script. The API compiles fine and then dies on boot with "Missing REALUNIT_W2W_GAS_LOW_BALANCE_THRESHOLD". The full .env.example already carries the variable with the same value, only the local template was missed. W2W transfers are inactive locally (DISABLED_PROCESSES=*), so the value only has to satisfy the check. 2. setup.js never loaded the .env it instructs the developer to create, so its own database checks fell back to the defaults in dbConfig(). With a customised SQL_* (for example a different port because 5432 is already in use) the script queried a different database than the API wrote to and aborted with the misleading "Database not ready after timeout" while the API was healthy. Other scripts in this repository already load dotenv the same way. Loading the .env before the safety checks also means ENVIRONMENT and SQL_HOST are now evaluated against the actual configuration instead of undefined defaults, which makes the existing production guard effective rather than weaker. * fix(dev): narrow the quick-start fix to the boot blocker Drop the dotenv change from this pull request and correct the comment. Loading .env in setup.js turned out to be the wrong scope here. The safety check allows remote hosts (/^sql-dfx-api-loc/i and /loc.*\.database\.windows\.net/i), while dbConfig() previously never saw SQL_HOST and therefore always worked on localhost. Reading .env would have let setAdminRole() and seedDepositAddresses() write to a remote database for the first time - a weaker guarantee than before. It also only solved half the problem, because docker-compose.yml and the script's API_URL keep their hard-coded port and would still not follow a customised .env, and dotenv is not a direct dependency of this package. Making the setup script configurable is a separate change that has to cover the compose file, the API URL and a tightened host allow-list together. This pull request stays on the one defect that blocks every developer. The comment justified the value with DISABLED_PROCESSES=*, which is wrong: that setting only suppresses cron processes, and the threshold is read in the HTTP path (realunit.service.ts). The real reason the value is inert locally is that assertW2wGasWalletFunded() rejects while the W2W wallet credentials are unset, before the threshold is ever compared. * fix(dev): say compared instead of read in the threshold comment The value is destructured before the credential check; only the comparison happens after it.
PR #4380 switched the `gh pr create` step in the Auto Release PR workflow from the workflow's own GITHUB_TOKEN to a personal access token, and lowered the job permission to `pull-requests: read`. Since then every release PR has carried a person's name as its author instead of the Actions bot, and the automation has depended on one account's credential lifetime. Revert both lines in this workflow. The permission has to go back to `write` because GITHUB_TOKEN needs it to open a PR; `gh pr list` in the earlier step is covered by it. The behavioural cost is documented at the call site rather than silently taken back: a PR opened with GITHUB_TOKEN does not start a `pull_request` workflow run for its `opened` activity, so the release PR opens without API PR CI, CodeQL Advanced, PR Review Bot, and API Migration Check. Later pushes to `develop` do start `synchronize` runs, so a release PR merged before any such push gets no `pull_request` run at all, while one that stays open across further merges does. This was measured on the bot-authored release PRs from before the PAT switch, not inferred: one merged unchanged had no `pull_request` run, another got its runs only once a later merge moved the head, and the PAT-created one got them four seconds after creation. The practical consequence for reviewers is in the comment too: a green release PR does not imply CI ran with `main` as the target. The guards PR #4380 removed from api-pr.yaml stay removed — restoring them would also suppress the `synchronize` runs, which are the only release-PR CI left.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 27, 2026 08:04
This was referenced Jul 27, 2026
The comment added in #4397 was wrong. It claimed a PR opened with GITHUB_TOKEN starts no `pull_request` workflow run at all, so the release PR would open with no CI. Checking the Actions history properly shows something different, and the practical advice that followed from it was misleading. What actually happens, verified against run attempt data: - The runs for the `opened` activity are created, but held. Attempt 1 completes as `action_required` with zero jobs, and a manual approval starts attempt 2, which is the one that executes. This follows from the repository's Actions approval policy (`all_external_contributors`), not from the token as such. - Only those runs are held. Later `synchronize` runs from human pushes to `develop` execute on attempt 1, and the PAT-authored release PRs did too. - While a release PR sits unapproved, `develop` push runs for the same head commit do execute and show up green next to the empty PR run records. So a freshly opened release PR shows a mix of pending, jobless entries and real push results. The comment now says that, and tells a reviewer what to do about it: approve the held runs, then confirm that what they are reading belongs to the `pull_request` runs for the current head. Only comments change; the executable configuration is byte-identical. Two earlier attempts at this note were also wrong, in the opposite direction each time. The wording is deliberately narrow now: it names the policy rather than the token, scopes the gate to the `opened` activity, and keeps the same-head push warning from the original note, which was the one part of it that was true.
TaprootFreak
approved these changes
Jul 27, 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: 3 new commit(s)
Checklist