Repository navigation
chore: raise the repo to spec-and-gate level (docs, CI, migrations, coverage) - #11
Merged
Merged
Conversation
Tool caches (.codegraph/.zcode/graphify-out/docs/superpowers/.video_agent) dirtied git status without belonging to the repository. ORM model backups, test_package build outputs and archived .cc backups duplicate tracked sources and can silently drift; test_404_fix.sh was a one-off debug script.
TECH_SPECS named a Controller/Plugin layering and Google Test while the code ships handlers/services/channels and DROGON_TEST; CLAUDE advertised an /api/v1/metrics/payments endpoint that never existed and unverifiable coverage/latency claims; CONTRIBUTING documented area-prefix commits that every recent commit ignores; troubleshooting pointed at --gtest_verbose.
…Auth2 residue AGENTS.md listed four skills that never existed and omitted two real entries; test-writer/reviewer/conventions docs taught gtest idioms the suite does not use; create-migration described authforge's Flyway chain; openapi-update pointed at an openapi.yaml that isn't there yet. All 8 .codex/workflows files and the oauth2_e2e_* scripts targeted a different repository outright — removed. New scripts/check_docs_drift.py gate keeps these invariants from rotting again, and scripts/clang_format.py makes the CI job, the agent hook and pre-commit agree on clang-format 22.
Centralize the first-party warning profile in cmake/Warnings.cmake (pay_apply_warnings) and gate -Werror//WX behind the opt-in DROGON_PAY_WERROR option, PRIVATE so installed consumers are unaffected. All three CI platforms now configure with it ON. The drogon_ctl-generated ORM models move to a drogon_pay_models OBJECT library compiled with the advisory profile only, since generated code must not be hand-edited to satisfy the bar (install(EXPORT) stays clean via $<TARGET_OBJECTS>). Silences the first-party C4100s the hard bar exposed: unused parameters in AlipayChannel::sendRequest, RefundService:: invokeRefundChannel, PaymentService::reconcileSummary, ConfigLoader::parseEnvVarName and HealthCheck error callbacks are now explicitly unnamed; behavior is unchanged.
One cross-platform build+test body parameterized by inputs (preset, runner, Conan settings, database on/off, per-suite ctest budget) instead of three copy-pasted workflows that had already drifted apart: the Linux lane applied sql/001+002 only and silently skipped 003/004, and its readiness loop `break`-ed out to a green step even when Postgres never came up. Both legs now probe with SELECT 1, hard-fail on timeout and dump the container log, and the migration loop walks sql/*.sql while skipping the 000_ dev-reset script by name. Service containers are replaced by `docker run -d` on Linux because a job `services:` block cannot be conditioned per matrix leg. Windows keeps its pwsh service-management steps verbatim — that block is the only place where a single shell would cost more than it buys. _sdk-smoke.yml generalizes conan-create.yml and adds the Linux leg, so the MSVC and GNU linkers both prove the packaged plugin still registers its routes. It asserts the "[test_package] OK" marker rather than the exit code alone, because can_run() skips the binary silently.
FAST (static-analysis, plus clang-tidy in parallel) -> MAIN (build-test x3) -> RELEASE (sdk-smoke x2), chained by `needs`, with concurrency cancelling superseded runs on the same ref. The three ruleset-required check names are reproduced verbatim via matrix.check_name, so branch protection keeps its teeth across the move. The legacy build-Drogon-from-source jobs are dispatch-only now and live in their own workflow instead of riding along in two platform files nobody triggers; ci-linux/ci-windows/ci-macos/conan-create are otherwise untouched and clearly marked as running their last cycle in parallel with ci.yml. Deleting them is the follow-up once this pipeline has been green alongside them on the same commit — that observation needs a push, so it is deliberately not taken here. AGENTS.md and ci-monitor.md now describe the gate layout and warn that renaming a check name silently drops merge protection.
The arm64 leg used to stop after `cmake --build`, so anything that only breaks on macOS (or only when the suite actually talks to a database) merged green. It now provisions Postgres/Redis itself: initdb into a throwaway cluster under $RUNNER_TEMP with -A trust, pg_ctl on 127.0.0.1:5432, a daemonized redis-server, the shared migration loop, then the same ctest entry the other platforms run. Trust auth is deliberate — the readiness probe and the CREATE ROLE/DATABASE bootstrap need a working connection before any password policy exists, and password auth is already exercised by the Linux and Windows legs. What this leg proves is that the arm64 build passes the suite, so it must not fail on provisioning mechanics. Revert this single commit to go back to build-only macOS; the check name `macos-build` is unchanged either way. This leg has never run a database in CI before, so watch it on the first push.
The HTTP surface had no machine-readable contract, so the docs had drifted from the code in ways that actively misled: amount was documented as an integer in cents while the service takes a decimal string in yuan, the state machine used SUCCESS where the code writes PAID, and api-documenter described /api/v1/* endpoints that were never registered. Add examples/pay-server/openapi.yaml as the single source of truth for 15 paths / 28 operations, with the documented quirks preserved rather than silently "fixed" (the FAIL vs FAILED spelling split between channels, the deprecated reconcile `date` parameter, notify callbacks answering the channel's own body format). Correct the consumers of that contract: TECH_SPECS state machines, docs/api/pay-api-examples.md, the ORM models README, the api-documenter agent, and the docker-integration-test skill in both mirrors.
A contract nobody checks is a second source of truth waiting to drift, which is exactly how the previous docs ended up describing endpoints that were never registered. scripts/check_openapi_routes.py closes that loop: it parses the registerHandler/ADD_METHOD_TO call sites (including the basePath_ concatenations, the qrPath variable and the ternary that pins /api/qrpay/create), diffs the resulting METHOD+path set against the spec in both directions, and compares the auth posture of each pair. Split into --lint-only and --routes-only so CI names the failing half, and written against the standard library only: the FAST gate must not be able to red a build because a runner image lacks PyYAML. The openapi-update skill documented "amounts are integers in fen" and pointed at a spec that did not exist yet. Rewritten around the gate and mirrored to .codex; AGENTS.md/CLAUDE.md now name the contract file.
Five consumers knew the migration file list and none of them could tell whether the database had caught up, so the list had already drifted in three separate directions: the per-platform Linux workflow applied two of the four versions, the deploy scripts globbed a sql/ directory that the plugin refactor had moved (applying nothing, successfully — and, before the move, running 000_drop_pay_tables.sql as if it were a version), and setup_database.bat shipped a default password. scripts/migrate_db.py is now the only executor: it discovers sql/NNN_*.sql, applies pending versions in order, commits each migration together with its schema_migrations row in one transaction, and refuses to run when a version already applied has changed bytes or when recorded tables were dropped out-of-band. --baseline adopts a database that compose's initdb.d built (only if the objects are really there), --status/--dry-run make the state readable, and --reset-schema replaces the hand-written drop recipe. Provisioning stayed out of it on purpose: the app role has no CREATEDB, so a DROP DATABASE could not be recovered by the same connection; the executor probes pg_database and prints the superuser command instead. scripts/check_migrations.py gates naming, an unbroken version chain, idempotence and non-destruction, with the content rules applied only to migrations that are not yet pinned in scripts/migrations_baseline.json — 001-004 predate the guard and are recorded as they are rather than retroactively held to a standard they would fail. Deploy and setup_database now call the executor, CI gained the hygiene step, and check_docs_drift.py rejects a doc that cites a migration version that no longer exists (the exact class of claim this change invalidates).
AGENTS.md documented a Windows script and a four-line conan+cmake incantation for everything else, so the Unix instructions were the ones you had to trust rather than run, and -debug only existed on Windows. build.sh and test.sh are now parameter-for-parameter twins of build.bat and test.bat (same flags, same ctest invocations, same artifacts), with new linux-debug and macos-debug presets behind their -debug, and the docs point at the scripts instead of at their expansion. test.bat lost its DB_HOST/DB_PORT/DB_NAME/DB_USER/DB_PASS=123456 block on the way: the suite reads its credentials from the .env build.bat copies next to the binary and no source file names those variables, so the block was a plaintext credential that did nothing. The version was declared in three places (CMakeLists.txt, conanfile.py, pay-admin package.json) and restated in six deploy/config comments, which is how it drifted. check_version_sync.py asserts the three declarations agree — now a static-analysis step — and its --tag mode additionally requires the tag to match the tree and CHANGELOG.md to already carry that section. The six restated version comments are gone; TECH_SPECS.md writes the rule down so the next one is a review comment, not a surprise. Every tracked examples/pay-server shell script also becomes mode 100755: the docs and the scripts' own headers show bare path invocations, which a 0644 checkout on Linux refuses.
release.yml validated a tag the way a developer types commands: it ran `conan create` on one platform and published whatever CHANGELOG section it found, so a tag that named a version the tree did not declare — or no version section at all — still produced a release. It now opens with a five-minute version-check job (`check_version_sync.py --tag "$GITHUB_REF_NAME"`, which also requires the CHANGELOG section to exist), then reuses _sdk-smoke.yml — the same Linux + Windows consumer workflow the RELEASE gate of ci.yml calls, so the tag path actually exercises the plugin routes instead of only linking once — and publish depends on both. /release was the other half of the same gap: its steps pointed at a library CMakeLists that only interpolates PROJECT_VERSION, at a PayFrontend that does not exist, at hand-running `gh release create` (which now collides with the pipeline's release) and, worst of all, at `git log ... > CHANGELOG.md`, which would have replaced the changelog with a commit dump. It is now the runbook for the gate above: archive the version section, bump the three declarations, verify locally with the same command CI runs, tag on master.
…scripts The deployment guide spelled Conan and preset commands out per platform (with macOS missing entirely) while telling the reader not to hand-write CMake commands, and the testing guide gave raw ctest lines with no mention of the wrappers. Both now name build.sh / build.bat and test.sh / test.bat first and keep the expanded Conan+preset sequence as the escape hatch for custom configurations, which is the same layering AGENTS.md describes.
… matches `test.bat -r Idempotency` parsed to `ctest -R "-r"`: CMD expands %~1 when it parses a parenthesized block, so the `shift` in the same block could never read the next argument. ctest matched nothing, printed "No tests were found!!!" and exited 0, and the wrapper printed "Tests passed successfully" - and its `if %errorlevel% neq 0` was expanded at parse time too, so it saw 0 regardless. Both scripts now drive `-l`/`-r` through the test binary, whose `-r` takes an exact DROGON_TEST case name and exits 1 when it finds none (tests/CMakeLists.txt registers one aggregate ctest case, so ctest cannot filter per case), and test.bat's flow is label-based so exit codes are read at run time. An all-tests run additionally refuses to call an empty suite a pass, and `-r` with no argument errors instead of running everything. Verified: suite green through test.bat (149 cases / 1452 assertions); `-r` on a real name exits 0, on a bogus name exits 1, `-r` alone exits 1; test.sh's paths exercised against a stub binary tree.
…oling refuse
--reset-schema: setup_database.{sh,bat} pass `--confirm-drop $PGDATABASE`, so the
repeated name was never a human decision, and a stray run against a remote host
dropped the schema of whichever database the credentials pointed at. The
executor now treats it as the loopback dev primitive it is; a remote host is
where staging and production live, and the way to move one forward is a
versioned sql/NNN_*.sql. Verified in-process with a stubbed psql: 127.0.0.1,
localhost and ::1 still issue the DDL, while a hostname, 10.x and 9.9.9.9 are
refused before any statement runs.
--write-missing: check_baseline exempts a pinned file from the content rules for
good, so pinning a fresh migration was a self-granted waiver - a new file
carrying DROP TABLE could walk itself through the gate. It now runs the naming
and content rules over exactly the candidates it is about to pin and writes
nothing when any of them fails.
…se job's rights check_version_sync matched with search(), so a second `project(... VERSION ...)` or `version = ...` line in the same file was invisible to it - the exact shape of "one source of truth" violation the guard exists to catch. It now matches with findall and refuses a file that states more than one version, treats a four-part `1.0.0.1` as a pattern/file drift instead of reading 1.0.0 out of it, and reports a missing CHANGELOG.md as a failure rather than a traceback. `--tag ""` fell back to the default mode and exited 0; a sentinel now separates an absent flag from an empty value. release.yml asked for contents: write at workflow level, which handed release rights to the checkout and smoke jobs that never publish anything: the default is now read, with write scoped to `publish`. Its awk interpolated the version into a regex (dots matching any character); it matches the heading literally instead and fails the job when the extraction yields nothing, so a blank release body cannot ship. Verified against a scratch tree: agreement passes, disagreement / a duplicate declaration / a nested package.json version / bad JSON / an empty or "v"-less tag all exit 1, and the extraction returns 3627 bytes for the shipped 1.0.0 section and 0 for an absent one (which the new guard turns into a failure).
environment_setup.md tells operators to put real credentials in .env.production, and the ignore list covered `.env`, `.env.local` and `.env.*.local` but not that file - so the documented workflow was one `git add .` away from committing a production password. The rule now covers the family and re-includes only the `.example` templates. Verified with git check-ignore: the four real env variants tracked nowhere, the four `.example` files still addable.
…nalysis MAIN depended on `static-analysis` alone, so clang-tidy ran next to the compilers and its verdict was advisory: a tidy regression turns its job red, but red on a check nothing depends on still merges, which is how the promoted batch would have been bypassed in practice. The `needs` edge is what makes it a blocker, and it does so independently of which contexts the branch ruleset happens to require. Validated the job graph by parsing the workflow: `static-analysis` and `clang-tidy` are both existing job ids, and the three required check names in `matrix.check_name` are unchanged.
… them `scripts/migrations_baseline.json` had been pinned from a CRLF working copy, so `001`/`003`/`004` carried digests of bytes that exist nowhere in the repository (the blobs are LF). Locally that is green; on a Linux runner the checkout is LF and rule 3 fails on the very first FAST step. This branch has never been pushed, so the defect would have been its first CI result. `.gitattributes` now declares the endings instead of inheriting each clone's core.autocrlf: `eol=lf` for text, `eol=crlf` for `.bat`/`.cmd` (cmd.exe needs it), binary for the assets whose bytes the guards hash. Normalising the working tree to match rewrote 195 files and zero index entries — `git diff` afterwards is empty, because the repository already stored every one of them as LF, which is the whole point. Two guards make the remaining confusion self-explanatory: rule 3 and the ledger check now say "these differ only in line endings" when the LF normalisation of the file reproduces the pinned digest, because "someone edited applied history" and "your checkout has CRLF" need opposite fixes. Verified on the real local ledger: `migrate_db.py --status` flags exactly 001/003/004 with the new hint (recorded d3fa6fe7… vs disk 80197c71…), and a CRLF copy of the whole sql/ directory turns every pin into that message rather than an immutability accusation. `check_migrations.py` passes on the re-pinned files, and a CRLF working copy of them is what the new branch covers. Docs: CONTRIBUTING states where endings come from and how to re-materialise an old clone, TECH_SPECS records that a pin is a digest of committed bytes, CHANGELOG logs both this and the `.env.production` gap the previous commit closed.
Two halves of the same supply-chain posture were only applied to the newest files. `coverage.yml`, `deploy.yml` and `secrets-scan.yml` still ran `actions/checkout@v4` and friends, i.e. a moving tag chosen by someone upstream on the day the job ran — including in the job that computes the coverage numbers and in the one that pushes images. Every reference is now a full commit SHA with the tag it came from in a comment; each SHA was resolved through the tag ref API and cross-checked against the pins already in use, so the repository has one spelling of `actions/checkout` and it is the commit `v4` names. The other half is the token. `ci.yml`, `_build-test.yml`, `_sdk-smoke.yml`, `coverage.yml` and `deploy.yml` now say `permissions: contents: read` at workflow level instead of accepting whatever the repository default happens to be; nothing in those jobs writes to GitHub (image pushes authenticate with DOCKER_*/AWS_* secrets, not the workflow token). `secrets-scan.yml` is deliberately left alone: gitleaks posts a commit status, and narrowing a security gate's grants without a run to observe is how it goes quiet rather than how it gets safer. No job logic changed. Validated by parsing all twelve workflows, and by the six static gates plus the route/version/migration guards, which are unaffected.
`.codex/` is a second agent's copy of `.claude/`, and three files had drifted without anyone noticing: db-operations kept the three-exemption list where the source has six and had no Mapper-construction section, project-conventions still put core logic in `pay-server` and still described four log levels, and orm-gen still cds into the pre-refactor path. A mirror is never re-read, so this class of drift has no failure mode short of an agent enforcing a rule the code stopped having. Rule 5 of check_docs_drift.py compares the intersection of the two trees byte for byte; only the intersection, because .claude/agents legitimately has no codex twin and codex keeps one .toml of its own. The source was wrong too, in the direction the branch is named for: the Mapper section named StringListCallback, AccessTokenCallback and RefreshTokenCallback — types this repository does not declare anywhere, left over from the project the template came from — and gave `(*sharedCb)(errorResult)` as the one propagation shape when the services use two. It now says what the code does: a guard per construction site, failure reported through the callback shape the call site already has (`reportMapperFailure` in CallbackService.cc, `OnceCallback::call` in the idempotency paths, `(*sharedCb)(...)` elsewhere). Verified in both directions: the guard passes on the re-mirrored tree, and a one-line injected drift in .codex/rules/data-access.md produced `[rule5 twin-parity] ... (455 vs 428 bytes)` with exit 1, clearing on restore.
tests/main.cc rewrote every listeners[] entry to the test port but left custom_config.pay.metrics_base_url at the value copied from the host config, which points at 5566. /metrics proxies to this process's own /metrics/base, so a case that scraped it would have collected a locally running dev PayServer's counters while believing they were its own.
…ealth /health is a deprecated alias whose own Sunset header already reads 2026-08-28, and it is the DB-backed probe either way; a rollout gate should name the endpoint it plans to keep. test.sh's usage text also stopped advertising -v as opt-in, since verbosity is already the default there.
The operations manual's multi-instance recipe passed --port/--config to a binary whose main() takes no arguments, so all three instances bound 5566; the deployment guide called a Debug build unlinkable, which the coverage pipeline disproves on every run it triggers; and four guides carried version/date stamps that were already three years stale. Each fix states where the fact comes from instead of restating it.
ctest registers one case, so the advertised `ctest -R Foo` filter matched nothing and exited 0; single-case work goes through the test binary's own -l/-r from the binary directory, which is also where config.json resolves. Version facts are now split into declaration sites, published-package references, and things that must not be restated, and the migration-history count says six drifted lists rather than three, re-checkable at git show 043c5ed^.
drogon-build launched PayServer with a -c flag it ignores, db-reset verified with a ctest filter that matches no case, and docker-integration-test asked for the test binary inside an image that ships only the server. Both mirrors change together, as rule 5 requires.
Rules 6 and 7 make this round's fixes permanent rather than a one-off sweep: a doc that restates a version or an update date is now rejected (16 such lines in the guides plus the two governance footers), and an entry script that grows only one platform half, an undeclared orphan, or a .sh indexed 100644 that a clone cannot execute fails the FAST gate. Each failure mode was exercised against a throwaway scripts directory.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…laceholder The drift guard's own CI debut was red, for two reasons that were both real. R2 asked whether a backticked path existed on disk, which a laptop answers yes to for build/, .env and certs/ and a checkout answers no to; it now asks the index and .gitignore, versioned inputs that cannot disagree. And the curl-auth-header rule reported every X-Api-Key: YOUR_API_KEY sample in the API examples as a leak, so that placeholder joins the allowlist the other documentation placeholders already use.
A clean-clone run of the guard stayed red on `build` and `certs` after the index resolution landed: `build/` and `certs/` are directory-only ignore patterns, and git will not apply one to a name it cannot establish is a directory — which on a CI checkout is precisely the situation. Probing the bare name and the trailing-slash spelling covers both, and the guard now answers identically on a laptop and in a clone with no build/, .env or certs/ on disk.
…phaned storeIdempotencySnapshot had no caller in either PaymentService.cc or RefundService.cc; snapshot persistence moved to IdempotencyService::updateResult when the write was ordered ahead of the response, and PaymentService's copy of toRfc3339Utc went the same way. GCC's -Wunused-function said so on the first Linux run of the DROGON_PAY_WERROR gate, which /W4 /WX never reports for an internal-linkage function. Nothing that persisted a snapshot was removed.
GCC's -Wunused-function and -Wunused-but-set-variable, neither of which MSVC's /W4 /WX implements, reported these on the Linux lane only. Three file-local OpenSSL RSA key-minting helpers (writeTempPrivateKey in the CreatePayment and RefundQuery suites, writePrivateKey in WechatPayClientTest) had no caller anywhere; the live minting is generateKeyAndCert. Their includes went with them, while the target-level OpenSSL 3.0 deprecation suppression stays for the callers that remain. Three cases fetched the callback's std::error_code into a local and never asserted on it, leaving the contract each case exists to pin unwatched: the degraded query-order and query-refund paths must report no service error, while the wechat-client-not-ready callback path must report 1400. All three assertions pass on the first run. 149 cases / 1455 assertions.
-Wunused-lambda-capture is clang-only and part of -Wall, so the macOS lane was the first to see it: the response callback of AlipaySandboxClient::sendRequest never touches a member. The timeoutMs_ read that looks like a use sits in sendRequest's own argument list, outside the lambda body.
-Wunused-lambda-capture is clang-only, so the macOS WERROR lane was the only gate that could see these. Sweeping the whole compile database, rather than trusting the first failing TU in the CI log, found fourteen in the service layer after the Alipay one; re-running to a fixpoint was required because four more surfaced only once the first ten were gone. No member access lived in any of those lambda bodies, so the captures were pure aliases: MSVC, the WSL g++ -Werror sweep and the local clang-tidy gate all agree nothing changed, and the suite still reports 149 cases / 1455 assertions.
CallbackService's constructor asserted non-null wechatClient_ and dbClient_, while PayPlugin::setTestChannels deliberately builds the service with either absent so the "wechat client not ready" and "Database client not available" branches are exercised. assert() is a no-op under NDEBUG, so every Release lane passed and only coverage.yml's Debug build aborted 2.4s into ctest - the same message reproduces from a local Debug tree. Both asserts are gone; the per-method null branches were always the real guard, and these were the only asserts left in the services layer. The same lane's macOS job died earlier still, on `brew install redis@7`, a formula homebrew-core has dropped, so the provisioning step failed before compiling anything. It installs `redis` now.
…n it The macOS leg burned its full 120 minutes inside `brew install` and never reached initdb: macos-14 is outside Homebrew's support window, so it has no bottles, so installing postgresql@15 and redis compiles from source and Redis 8 drags llvm@22 and rust in as build dependencies. Renaming the dropped `redis@7` formula was only ever going to buy a longer failure. The provisioning step is deleted and the leg is build-only, which is what its required check name `macos-build` has said since the earlier pass that renamed it off `macos-build-and-test` for having no databases, and what the reference repo's matrix does for macOS. The arm64 clang -Werror compile - the sole reporter of -Wunused-lambda-capture - needs no services, so the diagnostic coverage stays; AGENTS.md, the testing guide and the ci-monitor table stop claiming otherwise. coverage.yml went green for the first time (the Debug assert that aborted it 2.4s in is gone), and its log reports "no baseline yet - SEEDING ... written to scripts/coverage_baseline.json" into a working copy that dies with the runner, so every run would re-SEED and the ratchet would have no floor. The measured numbers are copied into a reviewed commit here (overall 40.21%, 3012/7490 lines, from run 35433146190 at this branch's HEAD), and the two docs that describe SEED now say the write is discarded and the commit is what turns the gate on.
… is green Run 35441152813 carried both pipelines to green on one commit: the new chain produced static-analysis, clang-tidy, the three MAIN legs and both sdk-smoke legs, while ci-linux / ci-windows / ci-macos / conan-create each reported success beside it. That is the parallel-verification cycle the migration note asked for, and nothing in the deleted set is unrecoverable - the required checks are the three matrix.check_name values (verified through the ruleset endpoint quoted in ci.yml's header), and `Conan Package` was never one of them. `conan create` does not lose PR-time coverage: the RELEASE gate runs it on Linux as well as Windows, where the Windows-only job it replaces ran only Windows. The README badges repointed at ci.yml / coverage.yml also stop pointing a renamed repository at its old owner. AGENTS.md, the ci.yml migration note, the _build-test.yml platform description and CHANGELOG stop describing files that no longer exist.
wangrh0909
approved these changes
Sep 19, 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
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.
What this is
A stack taking the repository from "docs and gates that drift" to the
specialisation level of the
authforgereference repo: every claim agovernance document makes is either checked by a CI gate or removed, and the
build/test/migrate/release surface is expressed once rather than copied.
Six themes, in commit order:
1f12c83..6241b5cDROGON_PAY_WERROR(generated ORM models split into an exempt OBJECT library), clang-tidy two-tier gate,tests/unitvstests/integrationsplit, opt-in gcov + per-directory coverage ratchetce7c688..8d51a6aci.ymlentry point (FAST → MAIN → RELEASE) over reusable_build-test.yml/_sdk-smoke.yml, required check names kept byte-identical, every action ref frozen to a SHA, every token grant declared0eab9ee,a2d9060043c5ed,01977a6,ac5b6f9scripts/migrate_db.py) replacing six drifted file lists, hygiene guard,.gitattributesso a Windows checkout stops breaking byte-hash gatesf7f0cfb,2d80b3c,af561521970a0f..a0be477What the review round changed
Docs that contradicted the code, each re-verified against the source before
being rewritten:
main()takes noargc/argv, yet theoperations manual,
CLAUDE.mdand thedrogon-buildskill passed--port/-cflags. The manual's four-instance recipe therefore bound 5566four times; it now runs one process per directory.
tests/CMakeLists.txtregisters a singlectest case, so
ctest -R Foomatches nothing and exits 0. Single-case worknow goes to the binary's own
-l/-rfrom the binary directory./healthanswersSunset: 2026-08-28, a datealready past, and both ops scripts plus the docker skill still polled it.
They use
/readyznow; deleting the alias itself is a breaking change thatbelongs to a version bump, so it is still served.
(
coverage.ymlbuilds Debug+gcov); the docker skill ran the test binaryinside an image containing only
/app/PayServerand/app/config.json.tests/main.ccrewrote listener ports but notcustom_config.pay.metrics_base_url, so a metrics scrape would have read alocally running dev server's counters while believing they were its own.
references / must-not-restate), because
check_version_sync.pyonly everreads the first — the six
drogon-pay/1.0.0doc references were beingmistaken for a broken guard.
Rules 6 (
no-version-stamps) and 7 (twin-scripts) inscripts/check_docs_drift.pylock the two classes that keep rotting: 18restamped doc lines were removed and an entry script may no longer exist for
only one platform, appear as an undeclared orphan, or be indexed
100644.Verification
ctest --test-dir build/windows-msvc -C Release→ 1/1 passed; the binaryitself reports 149 cases / 1455 assertions, all passed (the count rose by
three when the dropped assertions found below were restored).
check_architecture,check_test_layout,check_migrations,check_version_sync,check_openapi_routes,check_docs_drift(7 rules),clang_format --check(66 files).git show HEAD:of the four stamped guides (reportsexactly the 16 deleted lines) and empty on the cleaned tree.
(undeclared orphan, missing twin, non-executable
.sh), then deleted..env, key or credentialfile touched; the only literals are the disposable CI service-container
password and the gitleaks guard's own regex.
Found by CI after the first push
The stack's first real run caught three things the laptop could not see, which
is the argument for the gates existing:
backticked paths with
Path.exists(), so on this machine it accepted docspointing at
build/,.envandcerts/— all gitignored — and a checkouthas none of them. Now resolved from the git index plus
git check-ignore(probing both
pandp/, because directory-only patterns cannot be appliedto a path that does not exist). Verified in a clean
git clonewith noartefacts on disk, and by injecting fabricated paths there.
storeIdempotencySnapshotwas dead in two services, andPaymentService.cccarried an unusedtoRfc3339Utc. GCC's-Wunused-functionsaid so underDROGON_PAY_WERROR; MSVC's/W4 /WXnever reports an unreferenced internal-linkage function, so Windows and the
macOS lane were silent. Snapshot persistence lives in
IdempotencyService::updateResult, so these were leftovers of that refactor,not a missing write.
X-Api-Key: YOUR_API_KEYin the API examples was reported as a leak bycurl-auth-header, which matches on shape. The placeholder joined theallowlist the other documentation placeholders already use.
Rounds two and three: the defect classes only one compiler, or only Debug, can see
Each subsequent push failed for a reason the previous round's log could not
have shown, and in three cases the diagnosis belonged to exactly one of the
three compilers:
-Wunused-function/W4 /WXdoes not implement it)storeIdempotencySnapshotcopies + a deadtoRfc3339Utc, leftovers of the service refactor-Wunused-but-set-variablestd::error_codeand never asserted on it, so the degrade-vs-error contract they exist to pin was unwatched-Wunused-lambda-capture-Wall)thisthey never usedassertunderNDEBUGoffcoverage.ymlCallbackService's constructor asserted non-nullwechatClient_/dbClient_, whichPayPlugin::setTestChannelsand two tests deliberately violate to exercise the "not ready" branches; Release lanes were green because the assert is not compiledbrew install redis@7— the formula is gone from homebrew-core; renaming it toredisthen cost the whole 120-minute job timeout, becausemacos-14is outside Homebrew's support window and compiles Redis 8 (withllvm@22+rust) from sourceThat last row is the story of the macOS leg: the leg only needed a database
because this PR invented the requirement, and the runner image cannot supply
one. The provisioning step is deleted and the leg is build-only again, which is
what its required check name
macos-buildhas said since an earlier pass renamedit off
macos-build-and-testfor having no databases, and what the referencerepo's macOS matrix entry does. Nothing about the compiler-diagnostic coverage is
given up: the arm64 clang
-Werrorcompile needs no services, and the runtimesuite stays covered by the two legs that can stand a database up.
AGENTS.md,the testing guide and the ci-monitor table now say so instead of describing an
initdbcluster under$RUNNER_TEMPthat never ran.Two process lessons are recorded in the CHANGELOG rather than hidden: a CI log
stops at the first failing translation unit, so the same class had to be
hunted with a local whole-compile-database
g++ -fsyntax-only -Werrorsweep(WSL) and the same replay through
clang++ -fsyntax-only -Wall -Wextraratherthan waited for; and a
-Wunused-lambda-capturerun is not a complete inventory —after the first ten captures were stripped, the identical command reported four
more, so the sweep has to be re-run to a fixpoint.
The Debug finding was reproduced and re-verified locally in the existing
build/linux-coveragetree: the stale binary aborted with the exact CIassertion text, and after the fix
PayPlugin_WechatCallback_WechatClientNotReady(5 assertions) and
PayPlugin_WechatCallback_DbClientNotReady(4) pass underDebug+gcov. The full Debug suite still only runs in CI, because this machine's
WSL has no
testrole on its own Postgres.Known and deliberately out of scope
CREATE USER test WITH PASSWORD '123456'in_build-test.ymlputs apassword in argv, which the repository's own convention forbids. It is a
throwaway container credential, but the inconsistency is real and should go
through
PGPASSWORD.rather than defers the masking risk. Run 35441152813 had the new chain and
all four legacy checks green on the same commit, so the parallel-verification
cycle they were kept for is spent; before removing them the required-check set
was read back from the ruleset (only the three
matrix.check_namevalues, andConan Packagewas never one), and PR-timeconan createis covered by theRELEASE gate on two platforms instead of the legacy job's one.
schema_migrationsstill carries CRLF digests for 001/003/004 on thisdeveloper's database;
setup_database.batreplay fixes it locally and the CIlane is unaffected (loopback-only, non-blocking).
coverage.ymlwent green for the first time and its--ratchetstep reportedno baseline yet — SEEDING, writingscripts/coverage_baseline.jsoninto a working copy that dies with therunner — so every run re-seeds and the ratchet holds nothing. The numbers
that run measured (overall 40.21%, 3012/7490 lines, run 35433146190 at this
branch's HEAD) are committed here, CI-sourced but reviewed, and the next run
(35441152619) reported
Coverage ratchet passedwith no SEEDING line at thesame 3012/7490, so the gate is armed and the gcov numbers reproduce to the
line. The docs now say the SEED write is discarded.