ci: coverage/db-integration gates, dep boundaries, and workspace-scoping ratchet - #1199
Open
realcodesiman wants to merge 6 commits into
Open
realcodesiman wants to merge 6 commits into
realcodesiman wants to merge 6 commits into
Conversation
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.
Implements P0/P1/P2 items C1, C2, C3, C8, C5 (partial), B14 (partial safety net) from the system-design-review audit.
C1 — coverage gate in CI
Coverage was defined (80% threshold in the shared vitest preset) but CI never ran
--coverage. Added a starter-scope coverage job (packages/business+worker) rather than the full 56-suite matrix, to keep CI wall-clock reasonable.C2 — real Postgres/Redis integration tests in CI
packages/database/__tests__/integration/*alldescribe.skipIf(!databaseUrl)and never ran in CI. Added adatabase-integrationjob with realpostgres/redisservices (matchingdocker-compose.yml's image/credentials), migrating from zero then runningtest:db.C3 — dependency-cruiser no-direct-db rule
Added a
no-direct-db-importrule forbiddingapps/(builder|worker)andintegrations/*from importing@chatbotx.io/database/clientdirectly, plus wiredcheck:circular(madge) andcheck:unused(knip) into CI.C8 — zod dedupe
Root
package.jsonalready pinnedzod@^4.3.6via overrides;pnpm dedupecleaned up a remaining duplicate resolution andpnpm dedupe --checkwas added to CI so it can't silently regress.C5 — complexity lint rules at warn (partial)
Flipped
noExcessiveLinesPerFunction/noExcessiveCognitiveComplexityfromofftowarn(noterror— confirmed locally that warnings don't failultracite check's exit code). Surfaces 2310 warnings; not attempting to fix any of them here — that's the separate, larger god-file-splitting initiative the audit scoped as its own quarterly item.B14 — workspace-scoping safety net (partial)
Implemented the code-only safety net the audit proposed as an interim mitigation, explicitly not Postgres RLS (that needs its own staged rollout):
scripts/check-workspace-scoping.mjs: a TypeScript-AST scan ofpackages/database/src/repositories/**/*.tsfor functions that take aworkspaceIdparam but never reference it in a query-shaped body. Ratchets viacheck-workspace-scoping.allowlist.json— only fails CI on new violations. Found and allowlisted 2 real (verified false-positive) hits inads-conversion-rule/repository.ts, where the filter is built through a sharedworkspaceRuleFilterhelper the textual scan can't see through — documented with a comment at the helper.packages/database/__tests__/integration/workspace-isolation.test.ts: seeds two real workspaces' Contact/Conversation/Message rows and asserts a workspace-A-scoped read never returns workspace B's row, using the real repositories.Verification
pnpm --filter @chatbotx.io/database check-types && test:dbagainst real local Postgres — 19/19 integration tests pass (including the new isolation test)node scripts/check-workspace-scoping.mjspasses cleanly against the seeded baseline; smoke-tested that it does catch a deliberately-injected violationpnpm why zod/pnpm dedupe --checkverified clean