Stabilize the workspace and add clap-based CLI/env configuration - #1
Merged
stephane-segning merged 1 commit intoAug 9, 2026
Merged
Conversation
`just ci` could not pass, `next build` was broken, `cargo deny check` failed on a live advisory in a production dependency, and neither binary could be configured without editing source. This makes the scaffold actually start, run and stop correctly, without adding any payment capability it does not have. Configuration (the headline change): * Both binaries now parse a clap CLI where every option auto-resolves from an environment variable, with an explicit flag beating its env var: VPAY_BIND, DATABASE_URL, VPAY_PROFILE, VPAY_CONFIG, VPAY_PUBLIC_BASE_URL, RUST_LOG, VPAY_LOG_FORMAT and VPAY_SHUTDOWN_GRACE_SECONDS. Shared options live in a flattened `CommonArgs` so the two binaries cannot drift. * `--version` reports a real 0.1.0; the workspace was version 0.0.0. * `--profile` selects a config *file* and never a code path, per ADR-0003. Its doc comment says so explicitly. * `vpay-server` previously hardcoded 0.0.0.0:8080. Process lifecycle: * `vpay-server` now shuts down via `with_graceful_shutdown` on SIGINT or SIGTERM. It previously had to be SIGKILLed by `docker compose down`. `--shutdown-grace-seconds` bounds the drain and exits non-zero if the clock wins, so a forced cutoff is distinguishable from a clean drain. * `vpay-worker-bin` no longer exits immediately on boot, which an orchestrator reads as a crash loop. It stays up answering the same signals while logging a startup banner and a 60s heartbeat stating the job loop is not implemented and no jobs are being processed. Pipeline and build fixes: * `pnpm -r test` swept the Cypress package into the unit sweep, so `just ci` and CI's `web` job were structurally incapable of passing. `@vpay/e2e`'s script is now `e2e`; all callers updated. * `@vpay/ui` imported with explicit `.js` suffixes. TypeScript's `moduleResolution: "bundler"` maps those back to the `.ts` source, so `tsc` and Vitest passed while webpack took the suffix literally and `next build` failed outright. `pnpm -r build` now compiles all 8 packages. * `backends/Dockerfile` hardcoded `x86_64-unknown-linux-musl` and could never have built on an arm64 host; Alpine's toolchain is already musl-native, so it now builds the implicit host target. Both runtime stages run as non-root UID 65532. A `.dockerignore` was added. Supply chain — fixed by upgrading, not suppressing (`ignore = []`): * time 0.3.45 -> 0.3.47 for RUSTSEC-2026-0009, a *production* dependency. * testcontainers 0.23 -> 0.27 moves onto bollard 0.20, which drops rustls-pemfile (RUSTSEC-2025-0134) and replaces tokio-tar with the maintained astral-tokio-tar fork (RUSTSEC-2025-0111). * Internal path deps carry versions, clearing the wildcard ban. * rust-version 1.85 -> 1.88, derived from cargo metadata. NOT verified by compiling; rust-toolchain.toml records that caveat. schemas/vpay.cstack was written in a grammar that does not exist — Prisma syntax with an invented policy block, admitted as unverified in its own header. Rewritten against the real CrateStack grammar and verified with `cratestack check` (0.7.8). It remains excluded from the build graph and models only entities with a real, tested Rust type to mirror. Two documented claims were false and are corrected: docs/flows/ configuration.md and docs/flows/ledger.md both asserted database CHECK constraints that the schema grammar cannot express. Those invariants are enforced only in Rust. Deliberately NOT done, and recorded as such in docs/STATUS.md: no image was built and the compose stack was never brought up (Docker Hub is unreachable from the authoring environment); the Cypress specs have still never executed; the shutdown-grace timeout path has no live test, because the only route answers instantly and a slow test-only route would put a test double in the shipping router; `--database-url`, `--config` and `--public-base-url` are accepted but consumed by nothing. The 8 NotImplemented adapter tokens are untouched — vpay still cannot take a payment. Verified: `just ci` exits 0 (fmt-check, clippy -D warnings, both self-checks, 64 Rust tests / 5 ignored, typecheck, web tests, deny). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
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.
Summary
Makes the vpay scaffold actually start, run and stop correctly, and gives both binaries a real
clapCLI whose every option auto-resolves from an environment variable. No payment capability is added — the 8ProviderError::NotImplementedadapter tokens are untouched and vpay still cannot take a payment.just cicould not pass before this change, and not for a flaky reason:pnpm -r testswept the Cypress package into the unit test sweep, so the recursive runner always hitcypress runwith the binary deliberately not installed (CYPRESS_INSTALL_BINARY: 0in CI). Alongside that,next buildwas broken,cargo deny checkfailed on a live advisory in a production dependency, and neither binary could be configured without editing source.Source of truth. There is no tracking issue; this originated as a direct maintainer request. The load-bearing external references are the three advisories that drove the dependency work — RUSTSEC-2026-0009 (
time, a production dependency), RUSTSEC-2025-0134 (rustls-pemfile) and RUSTSEC-2025-0111 (tokio-tar) — plus the in-repo decisions this change is bound by: ADR-0003 (a profile selects a config file, never a code path), ADR-0004 (static musl), ADR-0006 (no test doubles in shipping processes) and ADR-0007.Intent
Six defects prevented the project from starting and working:
just ciwas structurally unpassable — the Cypress/unit sweep collision above. Not environment-dependent; arithmetically impossible.next buildwas broken and no test could see it.@vpay/uiimported with explicit.jssuffixes; TypeScript'smoduleResolution: "bundler"maps those back to the.tssource, sotscand Vitest were green while webpack took the suffix literally and failed to resolve.cargo deny checkfailed, including ontime— reachable from both shipping binaries, not just dev-dependencies.vpay-worker-binexited immediately on boot, which an orchestrator reads as a crash loop.vpay-serverhad no signal handling —docker compose downhad to SIGKILL it — and hardcoded0.0.0.0:8080.--shutdown-grace-secondswas parsed and consumed by nothing, advertising a bounded drain in--helpthat did not exist. Found by reading the code, not by any check.Scope
Configuration (headline).
clap4.6.6 withderive/env/wrap_help. Options:VPAY_BIND,DATABASE_URL,VPAY_PROFILE,VPAY_CONFIG,VPAY_PUBLIC_BASE_URL,RUST_LOG,VPAY_LOG_FORMAT,VPAY_SHUTDOWN_GRACE_SECONDS. An explicit flag beats its env var. Shared options live in a flattenedCommonArgsso the two binaries cannot drift, enforced by a test.--versionreports a real0.1.0(workspace was0.0.0).--profile's doc comment states outright that it selects a config file and never a code path.Process lifecycle.
vpay-servershuts down viawith_graceful_shutdownon SIGINT/SIGTERM;serve_with_bounded_drainraces the drain against a grace clock and exits non-zero if the clock wins, so a forced cutoff is distinguishable from a clean drain without parsing logs.vpay-worker-binstays up answering the same signals while logging a startup banner and a 60s heartbeat stating the job loop is not implemented.Pipeline/build.
@vpay/e2e's script renamedtest→e2ewith all four callers updated;.js-suffix imports fixed;backends/Dockerfileno longer hardcodesx86_64-unknown-linux-musl(Alpine's toolchain is already musl-native, so it builds the implicit host target — the old form could never build on an arm64 host); both runtime stages run as non-root UID 65532; new.dockerignore.Supply chain — fixed by upgrading, not suppressing.
deny.tomlstill hasignore = [].time0.3.45→0.3.47;testcontainers0.23→0.27 moves ontobollard0.20, which dropsrustls-pemfileentirely and replacestokio-tarwith the maintainedastral-tokio-tarfork. Internal path deps carry versions, clearing the wildcard ban.rust-version1.85→1.88.schemas/vpay.cstackwas written in a grammar that does not exist — Prisma syntax with an inventedpolicyblock, admitted as unverified in its own header because the vendor docs 404. Rewritten against the real CrateStack grammar. Notablypayment_method_types String[]was a hard parse error (no SQL bind representation for a list scalar on a DB-backed model), so the old file could never have parsed;policy ledger_is_append_only on Chargenamed the ledger but attached to a table that is not the ledger, and no ledger model existed at all despitevpay-ledgerhaving tested types.Two documented claims were false and are corrected.
docs/flows/configuration.mdanddocs/flows/ledger.mdboth asserted databaseCHECKconstraints that the grammar cannot express (@db_enforcepromotes only single-field validators; there is no cross-column@@check). Those invariants are enforced only in Rust.Verification
just ciexits 0 end to end —fmt-check→clippy -D warnings→ both self-checks → 64 Rust tests →typecheck→ web tests →deny.just cicargo nextest run --workspacepnpm -r testpnpm -r buildnext buildfailedcargo deny checkignore = []cratestack checkschema OKThe ignored-test count is unchanged at 5 — nothing was un-ignored to inflate a green run.
Screenshots / Evidence
Server started from environment variables only, no flags:
/healthz→200 ok; unknown route → the Stripe-shaped 404 envelope; SIGTERM → exit 0. The worker was confirmed still running after 3s (it previously exited instantly) and also exits 0 on SIGTERM. Both report0.1.0for--version.Risk Assessment
Low-to-moderate, and the risk is concentrated in what could not be verified.
docker pull alpine:3.22did not complete in five minutes; 4 of 5 required base images are uncached). The Dockerfiles were rewritten but never built.docs/STATUS.mdrecords them as "revised, still never built". A reviewer with network access should build them before trusting them.cargo metadata, never compiled against; 63 of 317 packages declare norust_version, so the true floor could be higher.rust-toolchain.tomlrecords this. The Dockerfile deliberately pinsrust:1.95.0-alpine3.22— the version actually known to build this workspace — rather than the unverified floor.grace_clock, but no live process test cuts off a genuinely slow request: the only route is/healthz, which answers instantly, and adding a slow test-only route would put a test double in the shipping router, which ADR-0006 andverify-no-mocksforbid.docs/STATUS.mdsays plainly the SIGTERM tests "would pass identically with the grace clock deleted."--database-url,--configand--public-base-urlare accepted but consumed by nothing — this is CLI plumbing, not the ADR-0003 config system. Their--helptext says so. On the worker,--shutdown-grace-secondsalso does nothing, and says so.Both repo self-checks (
verify-no-mocks,verify-status) pass, and the "vpay cannot take a payment / do not deploy it" banner remains prominent in bothREADME.mdanddocs/STATUS.md.AI Usage Declaration
AI (Claude Opus 5, via Claude Code) performed this work end to end: diagnosis, implementation across Rust/TypeScript/Docker/schema, and the documentation pass. Implementation was delegated to parallel Sonnet sub-agents on disjoint file sets; the orchestrating session independently re-ran every gate rather than accepting agent self-reports — which caught two defects the agents' own summaries did not surface (the inert
--shutdown-grace-secondsflag, and a proposedunsafeenv-mutation in tests that violates the workspace'sunsafe_code = "forbid"). One agent hypothesis I passed on (that missingexamples/*packages would break the frontend Docker install) was disproved by direct test and retracted.NotImplementedand remain declared indocs/STATUS.md.Reviewer Focus
backends/apps/vpay-server/src/main.rs— theserve_with_bounded_drain/grace_clockoneshot race, and thestd::process::exit(1)decision on timeout.docker buildboth targets and bring upcompose.yml -f compose.e2e.ymlon a machine with registry access.schemas/vpay.cstack— itsGAPcomments assert that certain invariants cannot be expressed in the grammar. If that is wrong, the corrections todocs/flows/configuration.mdanddocs/flows/ledger.mdare wrong too.docs/STATUS.md— is anything marked ✅ that would not fail a test if it broke?