Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,13 @@ jobs:
- test-workers
- test-web
- build
# e2e was NOT in this list, so a red E2E run left `verify` green. Since
# verify is the job branch protection keys off, that made the whole E2E
# suite advisory without anyone deciding it should be — a gate that runs,
# reports, and cannot block. It costs verify the wait for e2e (~5 min,
# and it is the longest job), which is the price of it meaning what its
# name says.
- e2e
steps:
# Block scalar, not a plain one: a plain YAML scalar may not contain
# ": " and `echo "results: …"` does, which is a parse error rather than
Expand Down Expand Up @@ -388,6 +395,14 @@ jobs:
- name: E2E tests (Playwright, seeded D1)
run: npm run test:e2e

# Second run, second config: the multi-user seed writes users into the
# standalone tenant, which 409s the first run's fresh-setup assertion, so
# the two cannot share one D1. Same worker (reuseExistingServer), so this
# is one extra globalSetup, not one extra boot. See
# playwright.seeded.config.ts for the full reasoning.
- name: E2E tests (multi-user seed — subsystem D/E)
run: npm run test:e2e:seeded

- name: Upload Playwright report on failure
if: failure()
uses: actions/upload-artifact@v4
Expand Down
3 changes: 3 additions & 0 deletions knip.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@
"worker-configuration.d.ts",
"**/*.d.ts"
],
"tags": [
"-gateConsumed"
],
"ignoreBinaries": [
"tsgo"
],
Expand Down
48 changes: 32 additions & 16 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
"test:workers": "vitest run --config vitest.workers.config.ts",
"test:types": "cross-env NODE_OPTIONS=--max-old-space-size=8192 vitest run --typecheck --config vitest.typecheck.config.ts",
"test:e2e": "playwright test",
"test:e2e:seeded": "playwright test -c playwright.seeded.config.ts",
"db:migrate": "node scripts/wrangler.mjs d1 migrations apply DB --local",
"seed:local": "node scripts/seed-local-e2e.mjs",
"db:migrate:remote": "node scripts/wrangler.mjs d1 migrations apply DB --remote",
Expand Down Expand Up @@ -196,6 +197,7 @@
"@hono/node-server": ">=2.0.5",
"brace-expansion": "^5.0.9",
"ip-address": ">=10.2.2",
"fast-uri": "^3.1.5",
"postcss": ">=8.5.18",
"valibot": ">=1.4.2"
},
Expand Down
26 changes: 13 additions & 13 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,16 +189,13 @@ export default defineConfig({
testMatch: 'estimate-range.spec.ts',
dependencies: ['api'],
},
{
// Sprint 2 regression — Track A fixes (A1-A4).
name: 'sprint2-regression',
testMatch: 'sprint2-regression.spec.ts',
},
{
// R7-06 — public booking page native date input.
name: 'booking-date-input',
testMatch: 'booking-date-input.spec.ts',
},
// `sprint2-regression` and `booking-date-input` projects deleted
// (2026-08 skip-debt clearance) together with their spec files. Every
// test in both was inside a `describe.skip` scraping Alpine-era source
// files the RR migration removed (src/templates/**, public/js/auth.js)
// or driving the Alpine booking form. A project whose testMatch
// resolves to nothing is a new way to report green over zero tests, so
// the entries go with the files.
{
// env-guarded (R8 fix): matches nothing by default so the dead
// 'cloud' project no longer silently swallows via testIgnore —
Expand Down Expand Up @@ -274,10 +271,13 @@ export default defineConfig({
// Destructive (reset/restore DB) — env-gated inside the specs:
{ name: 'backup-restore-seed', testMatch: 'backup-restore-seed.spec.ts' },
{ name: 'backup-restore-verify', testMatch: 'backup-restore-verify.spec.ts' },
// DS-0520 subsystem C/D/E — skip-shells pending multi-user seed harness:
// DS-0520 subsystem C — still a skip-shell, and for a reason nothing in
// this repo can supply: two servers plus the Stripe CLI. See the spec.
{ name: 'subsystem-c-stripe-smoke', testMatch: 'subsystem-c-stripe-cross-repo-smoke.spec.ts' },
{ name: 'subsystem-d-flows', testMatch: 'subsystem-d-flows.spec.ts' },
{ name: 'subsystem-e-flows', testMatch: 'subsystem-e-flows.spec.ts' },
// subsystem-d-flows / subsystem-e-flows moved to playwright.seeded.config.ts.
// They need the multi-user seed, which writes users into the standalone
// tenant and therefore 409s the `api` project's fresh-setup assertion —
// the two cannot share one D1. `npm run test:e2e:seeded` runs them.
// Commercial PCA Task 19a — real TOC page numbers (two-pass Chrome +
// pdf-lib). Exercises the actual worker report render + BROWSER binding;
// see tests/e2e/report-toc-numbers.spec.ts for its harness requirements.
Expand Down
44 changes: 44 additions & 0 deletions playwright.seeded.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { defineConfig } from '@playwright/test';
import base from './playwright.config';

/**
* The multi-user-seed E2E run: `npm run test:e2e:seeded`.
*
* WHY THIS IS A SECOND CONFIG AND NOT TWO MORE PROJECTS IN THE FIRST ONE.
*
* `tests/seed-fixtures.ts` writes its users into TENANT_A, and TENANT_A *is*
* the standalone workspace (`SINGLE_TENANT_ID`) — standalone login resolves no
* other tenant, so the fixtures cannot live anywhere else. But the default
* run's `api` project opens with API-01 asserting that
* `POST /api/auth/setup` returns a FRESH 200, and setup 409s
* (`already_initialized`) the moment any user has a tenant. The seed and that
* assertion are mutually exclusive by construction, in one shared D1.
*
* So they are two runs, not two projects: the default suite stays unseeded and
* `api` still proves a cold install works, and this config seeds and runs the
* subsystem specs that need the seeded workspace. Both drive the SAME worker
* (`reuseExistingServer`), so running them back to back costs one boot.
*
* The alternative — leaving these specs in the default config and skipping them
* unless SEED_E2E is set — was rejected: it puts nine tests back in the
* permanently-skipped column that this split exists to empty.
*/

// Read by tests/global-setup.ts. Set here rather than in an npm script so the
// command is identical on Windows and CI (no `cross-env`, no shell-specific
// `VAR=x cmd` prefix, which PowerShell does not parse).
process.env.SEED_E2E = '1';

export default defineConfig({
...base,
projects: [
{
name: 'subsystem-d-flows',
testMatch: 'subsystem-d-flows.spec.ts',
},
{
name: 'subsystem-e-flows',
testMatch: 'subsystem-e-flows.spec.ts',
},
],
});
27 changes: 19 additions & 8 deletions scripts/check-deadcode.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,29 @@
* entries that no longer hit are informational only (run `--update` to prune).
*
* Why a wrapper (and not plain `knip`): knip 6 has no native baseline file, and
* the tree carries a large pre-existing residual of dead exports/files/deps
* (leftovers the T9 deletion pass did not reach). Freezing that residual lets
* the gate go green today while catching every NEW piece of dead code a future
* change introduces — the same baseline-ratchet used by `check-tenant-scoping`
* and `check-file-size`.
* the tree once carried a large residual of dead exports/files/deps (leftovers
* the T9 deletion pass did not reach). Freezing that residual let the gate go
* green while catching every NEW piece of dead code — the same baseline-ratchet
* used by `check-tenant-scoping` and `check-file-size`.
*
* The ratchet has since reached zero: `scripts/knip-baseline.json` is `[]` and
* MUST stay that way. An empty baseline means every knip finding is a failure,
* which is the whole point — a baseline that only ever grows has stopped being
* a ratchet.
*
* node scripts/check-deadcode.mjs # gate (CI + `npm run lint`)
* node scripts/check-deadcode.mjs --update # regenerate the baseline snapshot
*
* The baseline (`scripts/knip-baseline.json`) IS the documented allow-list of
* known residual dead code. Shrinking it (deleting real dead code, then running
* `--update`) is always safe and encouraged; growing it requires review.
* When a NEW finding is legitimate, the fix is to declare WHY in `knip.json`,
* not to grow the baseline. The three shapes that come up:
* - a file that is a graph root (a worker/config/route entry) -> `entry`
* - an export a NON-TypeScript tool consumes (a gate script that reads the
* source as text, so the module graph cannot see it) -> tag the export
* `@gateConsumed` (wired via `tags: ["-gateConsumed"]`); this is per-export,
* so the rest of the file stays under the gate. Example:
* `ERASURE_OUT_OF_SCOPE`, read by `scripts/check-erasure-manifest.mjs`.
* - a dependency knip cannot resolve -> `ignoreDependencies`
* `--update` is a last resort and needs review of every entry it adds.
*
* console.* is intentional — this is a build script, not server code (the
* no-console rule is server-only).
Expand Down
2 changes: 1 addition & 1 deletion scripts/file-size-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"app/routes/settings-communication-templates.tsx": 525,
"server/api/portal.ts": 525,
"server/services/portal-access.service.ts": 525,
"server/api/inspections/publish.ts": 520,
"server/api/inspections/publish.ts": 521,
"server/api/bookings/agreement.ts": 519,
"app/components/settings/ManagedComplianceWizard.tsx": 514,
"server/services/invoice.service.ts": 513,
Expand Down
5 changes: 1 addition & 4 deletions scripts/knip-baseline.json
Original file line number Diff line number Diff line change
@@ -1,4 +1 @@
[
"exports:server/lib/compliance/erasure-manifest.ts:ERASURE_OUT_OF_SCOPE",
"types:server/lib/compliance/erasure-manifest.ts:ErasureOutOfScopeEntry"
]
[]
10 changes: 1 addition & 9 deletions scripts/status-literal-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,5 @@
"app/lib/dashboard-filters.ts::reportStateLabel::reportStatus === \"in_progress\"",
"app/lib/dashboard-filters.ts::reportStateLabel::reportStatus === \"published\"",
"app/lib/dashboard-filters.ts::reportStateLabel::reportStatus === \"submitted\"",
"app/routes/agent/dashboard.tsx::AgentDashboardPage::reportStatus === \"published\"",
"server/api/inspections/publish.ts::renderBoth::reportStatus: 'in_progress'",
"server/api/inspections/publish.ts::renderBoth::reportStatus: 'submitted'",
"server/lib/google-calendar.ts::syncEventsToGcal::status === 'completed'",
"server/portal/outbox.service.ts::publishRow::status: 'published'",
"server/services/concierge.service.ts::approveByInspector::status: 'confirmed'",
"server/services/event.service.ts::createEvent::status: 'scheduled'",
"server/services/event.service.ts::updateEventStatus::status === 'cancelled'",
"server/services/event.service.ts::updateEventStatus::status === 'completed'"
"app/routes/agent/dashboard.tsx::AgentDashboardPage::reportStatus === \"published\""
]
7 changes: 4 additions & 3 deletions server/api/inspections/publish.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { createApiResponseSchema, SuccessResponseSchema } from '../../lib/valida
import { PublishInspectionSchema, CreateReinspectionSchema, CancelInspectionSchema } from '../../lib/validations/inspection.schema';
import { inspections as inspectionTable } from '../../lib/db/schema';
import { INSPECTION_STATUS } from '../../lib/status/inspection-status';
import { REPORT_STATUS } from '../../lib/status/report-status';
import { fireAutomation } from '../../services/inspection/shared';
import { eq, and } from 'drizzle-orm';
import { getTenantId, getDrizzle } from '../../lib/route-helpers';
Expand Down Expand Up @@ -494,14 +495,14 @@ const publishRoutes = createApiRouter()
const { id } = c.req.valid('param');
const result = await runReportTransition(() => c.var.services.inspection.submitReport(id, tenantId), 'Failed to submit report');
if (!result.ok) return c.json({ success: false as const, error: { code: 'BAD_REQUEST', message: result.message } }, 400);
return c.json({ success: true as const, data: { reportStatus: 'submitted' } }, 200);
return c.json({ success: true as const, data: { reportStatus: REPORT_STATUS.SUBMITTED } }, 200);
})
.openapi(returnReportRoute, async (c) => {
const tenantId = getTenantId(c);
const { id } = c.req.valid('param');
const result = await runReportTransition(() => c.var.services.inspection.returnReport(id, tenantId), 'Failed to return report');
if (!result.ok) return c.json({ success: false as const, error: { code: 'BAD_REQUEST', message: result.message } }, 400);
return c.json({ success: true as const, data: { reportStatus: 'in_progress' } }, 200);
return c.json({ success: true as const, data: { reportStatus: REPORT_STATUS.IN_PROGRESS } }, 200);
})
.openapi(unpublishReportRoute, async (c) => {
const tenantId = getTenantId(c);
Expand All @@ -513,7 +514,7 @@ const publishRoutes = createApiRouter()
// report the inspector has withdrawn. Expire all of this inspection's
// access tokens immediately.
await c.var.services.portalAccess.setExpiryForInspection(tenantId, id, Date.now());
return c.json({ success: true as const, data: { reportStatus: 'in_progress' } }, 200);
return c.json({ success: true as const, data: { reportStatus: REPORT_STATUS.IN_PROGRESS } }, 200);
});

export default publishRoutes;
9 changes: 9 additions & 0 deletions server/lib/compliance/erasure-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,22 @@ export const ERASURE_MANIFEST: ErasureRule[] = [
* A PII-heuristic column the manifest DELIBERATELY does not act on. Every entry
* must say why — the reason is what a DSAR audit reads, and the CI gate
* (`scripts/check-erasure-manifest.mjs`) hard-fails an entry without one.
*
* @gateConsumed `scripts/check-erasure-manifest.mjs` reads this declaration out
* of the SOURCE TEXT (`arrayBody(src, 'ERASURE_OUT_OF_SCOPE')`) rather than
* importing it — the gate is a plain .mjs script and the manifest is TypeScript.
* That consumption is invisible to a module-graph analyzer, so knip would report
* both symbols as dead. The tag (knip `tags: ["-gateConsumed"]`) says "a tool
* consumes this", which is true; a dead-code baseline entry would have said
* "this is dead and we tolerate it", which is not.
*/
export interface ErasureOutOfScopeEntry {
table: string;
column: string;
reason: string;
}

/** @gateConsumed read as source text by `scripts/check-erasure-manifest.mjs`. */
export const ERASURE_OUT_OF_SCOPE: ErasureOutOfScopeEntry[] = [
// Columns that ride with a row-delete rule above (per-column scan cannot
// see row semantics).
Expand Down
3 changes: 2 additions & 1 deletion server/lib/db/schema/tenant/integration.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { sqliteTable, text, integer, index } from 'drizzle-orm/sqlite-core';
import { tenants } from './core';
import { users } from './user';
import { SYNC_OUTBOX_STATUS, SYNC_OUTBOX_STATUSES } from '../../../status/sync-outbox-status';

/**
* Outbox for core → portal sync events. Append happens
Expand All @@ -20,7 +21,7 @@ export const syncOutbox = sqliteTable('sync_outbox', {
eventType: text('event_type').notNull(),
payload: text('payload').notNull(),
// Schema Rules: state-machine column declares its enum (type-layer only).
status: text('status', { enum: ['pending', 'published', 'failed'] }).notNull().default('pending'),
status: text('status', { enum: [...SYNC_OUTBOX_STATUSES] }).notNull().default(SYNC_OUTBOX_STATUS.PENDING),
attempts: integer('attempts').notNull().default(0),
createdAt: integer('created_at', { mode: 'timestamp_ms' }).notNull(),
lastTriedAt: integer('last_tried_at', { mode: 'timestamp_ms' }),
Expand Down
3 changes: 2 additions & 1 deletion server/lib/google-calendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import { drizzle } from 'drizzle-orm/d1';
import { eq } from 'drizzle-orm';
import { logger } from './logger';
import { EVENT_STATUS } from './status/event-status';

export const GOOGLE_AUTH_URL = 'https://accounts.google.com/o/oauth2/v2/auth';
export const GOOGLE_TOKEN_URL = 'https://oauth2.googleapis.com/token';
Expand Down Expand Up @@ -161,7 +162,7 @@ export async function syncEventsToGcal(

for (const ev of events) {
// Skip cancelled / completed events — they shouldn't appear on the calendar.
if (ev.status === 'cancelled' || ev.status === 'completed') {
if (ev.status === EVENT_STATUS.CANCELLED || ev.status === EVENT_STATUS.COMPLETED) {
skipped++;
continue;
}
Expand Down
26 changes: 26 additions & 0 deletions server/lib/status/sync-outbox-status.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Single source of truth for the SYNC OUTBOX (core -> portal delivery) axis.
*
* This is a DIFFERENT axis from `REPORT_STATUS`, and the collision is the whole
* reason this file exists: an outbox row's `published` means "handed to the
* queue", while a report's `published` means "delivered to the client". They
* share one English word and nothing else — no consumer of one should ever be
* able to reach for the other's constant and typecheck.
*
* State machine (spec 6): pending -> published (terminal happy path). `failed`
* is set ONLY by the DLQ writeback. Legacy `done` rows are treated as terminal
* and ignored by the sweeper — deliberately absent here, since nothing may
* WRITE it.
*
* Every consumer (drizzle enum, service writes, queries) MUST derive from these
* — no bare status string literals.
*/
export const SYNC_OUTBOX_STATUSES = ['pending', 'published', 'failed'] as const;

type SyncOutboxStatus = typeof SYNC_OUTBOX_STATUSES[number];

export const SYNC_OUTBOX_STATUS = {
PENDING: 'pending',
PUBLISHED: 'published',
FAILED: 'failed',
} as const satisfies Record<string, SyncOutboxStatus>;
Loading
Loading