Skip to content

docs: document layer separation, duplications, and sync risks#101

Open
michael-88944 wants to merge 2 commits into
mainfrom
docs/architecture-analysis
Open

docs: document layer separation, duplications, and sync risks#101
michael-88944 wants to merge 2 commits into
mainfrom
docs/architecture-analysis

Conversation

@michael-88944
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Member

@ilyar ilyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The document says pnpm stdb:generate guards both TMA and payments bindings. Today the root script regenerates only apps/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.

  2. The document says Telegram initData validation in apps/server and apps/payments is identical. It is not. The payments version is stricter: timing-safe hash compare and required valid auth_date; the legacy server version uses direct string comparison and treats auth_date as optional. Please phrase this as duplicated purpose with different implementations, and make payments the current reference.

  3. packages/shared is 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/shared is 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
@michael-88944 michael-88944 requested a review from ilyar May 22, 2026 14:33
@michael-88944
Copy link
Copy Markdown
Contributor Author

All four blocking issues are fixed. Here's what changed in docs/about-project.md:

  1. stdb:generate scope — The "Generate bindings" section now explicitly states pnpm stdb:generate only covers apps/tma, and shows the separate spacetime generate command for payments bindings, with a note that both must run after every schema change.

  2. Telegram initData — "The logic is identical" is replaced with an accurate description: payments uses timing-safe comparison + required auth_date; server uses direct === + optional auth_date. Payments is named the current reference.

  3. packages/shared framing — "correct and only place for game rules" is replaced with "correct place for the shared frontend/test game rules layer", with an explicit note that production gameplay is server-authoritative and backend rules must stay in sync manually.

  4. Schema/bindings rule — The sync risks table row now names both generate commands, and a new paragraph states the explicit rule: every schema change regenerates all consumer bindings, or the exception is justified in the PR description.

Copy link
Copy Markdown
Member

@ilyar ilyar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed after commit 2c4a55d, also in context with PR #103.

Most of the previous blockers are resolved:

  • pnpm stdb:generate is now documented as TMA-only;
  • payments bindings now have an explicit separate generate command;
  • Telegram initData validation is accurately described as same purpose but different implementations, with payments as the reference;
  • packages/shared is 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants