docs: document layer separation, duplications, and sync risks#101
docs: document layer separation, duplications, and sync risks#101michael-88944 wants to merge 2 commits into
Conversation
ilyar
left a comment
There was a problem hiding this comment.
Reviewed in context with PR #102 and PR #103. Direction is useful, but this should not be merged until a few architecture statements are corrected.
Blocking issues:
-
The document says
pnpm stdb:generateguards both TMA and payments bindings. Today the root script regenerates onlyapps/tma/src/module_bindings; payments bindings are not covered by that command. This can mislead future SpacetimeDB schema work. Please either document both explicit generate commands or add a root script that regenerates all consumers. -
The document says Telegram
initDatavalidation inapps/serverandapps/paymentsis identical. It is not. The payments version is stricter: timing-safe hash compare and required validauth_date; the legacy server version uses direct string comparison and treatsauth_dateas optional. Please phrase this as duplicated purpose with different implementations, and make payments the current reference. -
packages/sharedis described as the correct and only place for game rules. That is too strong for the current architecture. Production gameplay is server-authoritative in SpacetimeDB;packages/sharedis the shared frontend/test rules layer, while backend rules are duplicated and must stay synchronized.
Given PR #103, please also make the schema/bindings rule explicit: every SpacetimeDB schema change must regenerate bindings for every active consumer, or explicitly justify why a consumer should not receive updated bindings.
- stdb:generate only covers apps/tma; add explicit payments generate command and note both must run after every schema change - Telegram initData: replace "identical" with accurate diff — payments uses timing-safe compare + required auth_date; server uses === + optional auth_date; payments is the reference - packages/shared: soften "correct and only place" to "shared frontend/test rules layer"; production gameplay is server-authoritative in SpacetimeDB - Add explicit schema/bindings rule: every schema change must regenerate all consumer bindings or justify the exception in the PR
|
All four blocking issues are fixed. Here's what changed in docs/about-project.md:
|
ilyar
left a comment
There was a problem hiding this comment.
Re-reviewed after commit 2c4a55d, also in context with PR #103.
Most of the previous blockers are resolved:
pnpm stdb:generateis now documented as TMA-only;- payments bindings now have an explicit separate generate command;
- Telegram
initDatavalidation is accurately described as same purpose but different implementations, with payments as the reference; packages/sharedis now described as the frontend/test shared rules layer, while SpacetimeDB remains production-authoritative;- the schema/bindings rule now directly supports the remaining PR #103 blocker: schema changes must regenerate every active consumer binding or document an intentional exception.
One remaining docs correctness issue before merge:
In the Layer Violation Checks section, the sentence still says: "All three apps consume @elmental/shared only through the workspace package name." That contradicts the dependency graph just above, where apps/payments is correctly documented as not depending on @elmental/shared. Please change it to something like: "apps/tma and apps/server consume @elmental/shared only through the workspace package name; apps/payments does not depend on it."
Synergy note: once that line is fixed, I think #101 should merge before #103 because it gives future reviewers/agents the exact schema-binding and P2E boundary checks that #103 currently needs.
No description provided.