2026-08-21 17:39:25 +05:30
|
|
|
# Specification Quality Checklist: SaaS Product Integration & Inbound Request Trust
|
|
|
|
|
|
|
|
|
|
**Purpose**: Validate specification completeness and quality before proceeding to planning
|
|
|
|
|
**Created**: 2026-08-21
|
|
|
|
|
**Feature**: [spec.md](../spec.md)
|
|
|
|
|
|
|
|
|
|
## Content Quality
|
|
|
|
|
|
|
|
|
|
- [x] No implementation details (languages, frameworks, APIs)
|
|
|
|
|
- [x] Focused on user value and business needs
|
|
|
|
|
- [x] Written for non-technical stakeholders
|
|
|
|
|
- [x] All mandatory sections completed
|
|
|
|
|
|
|
|
|
|
## Requirement Completeness
|
|
|
|
|
|
|
|
|
|
- [x] No [NEEDS CLARIFICATION] markers remain
|
|
|
|
|
- [x] Requirements are testable and unambiguous
|
|
|
|
|
- [x] Success criteria are measurable
|
|
|
|
|
- [x] Success criteria are technology-agnostic (no implementation details)
|
|
|
|
|
- [x] All acceptance scenarios are defined
|
|
|
|
|
- [x] Edge cases are identified
|
|
|
|
|
- [x] Scope is clearly bounded
|
|
|
|
|
- [x] Dependencies and assumptions identified
|
|
|
|
|
|
|
|
|
|
## Feature Readiness
|
|
|
|
|
|
|
|
|
|
- [x] All functional requirements have clear acceptance criteria
|
|
|
|
|
- [x] User scenarios cover primary flows
|
|
|
|
|
- [x] Feature meets measurable outcomes defined in Success Criteria
|
|
|
|
|
- [x] No implementation details leak into specification
|
|
|
|
|
|
|
|
|
|
## Notes
|
|
|
|
|
|
|
|
|
|
- Auth mechanism choice (signed tokens/OAuth2/mTLS) and rotation-window length are deliberately
|
|
|
|
|
left to `/speckit-plan`, not decided here — see spec.md Assumptions.
|
|
|
|
|
- Idempotency-key enforcement is explicitly deferred to the future ticketing feature (FR-012
|
|
|
|
|
reserves the field only); this is a scope boundary, not a gap.
|
|
|
|
|
- Exact rate-limit values and auth-mechanism-per-integration defaults are
|
|
|
|
|
`REQUIRES BUSINESS CONFIRMATION` per docs/10-implementation-roadmap.md — not invented here.
|
|
|
|
|
- All items pass; no revision iterations were needed.
|
2026-08-21 18:40:43 +05:30
|
|
|
|
|
|
|
|
## Implementation notes (added during /speckit-implement)
|
|
|
|
|
|
|
|
|
|
- **Found and fixed a codebase-wide bug, not specific to this feature**: `src/app.ts` called
|
|
|
|
|
`app.setErrorHandler(...)` *after* `bootstrapRoutes(app)` had already registered every domain
|
|
|
|
|
module's routes. Fastify resolves each encapsulated child context's error handler at the time
|
|
|
|
|
that context is registered — a handler set on the parent afterwards does not retroactively
|
|
|
|
|
apply to already-registered children. Every module registered via `app.register(someRoutes)`
|
|
|
|
|
(which is every module in this codebase, since none use `fastify-plugin`) was silently falling
|
|
|
|
|
back to Fastify's default `{statusCode, error, message}` error shape instead of this app's
|
|
|
|
|
`{success:false, error:{code,message,details}, requestId}` envelope, for *any* error — not
|
|
|
|
|
just ones from this feature's plugin. Fixed by moving `setErrorHandler`/`setNotFoundHandler`
|
|
|
|
|
before `bootstrapRoutes` in `src/app.ts`. Covered by a new regression test in
|
|
|
|
|
`tests/unit/app.test.ts` (verified it fails without the fix, passes with it).
|
|
|
|
|
- **Found and fixed a second bug in the same handler**: the generic (non-`AppError`,
|
|
|
|
|
non-`ZodError`) fallback branch always returned `500`, even for framework-level errors that
|
|
|
|
|
already carry their own client-facing `statusCode` (e.g. Fastify's body-parser rejecting
|
|
|
|
|
malformed JSON is a `400`, not a server failure). Fixed to preserve the original
|
|
|
|
|
`statusCode`/`code` when it's in the 4xx range.
|
|
|
|
|
- **Found and fixed a pre-existing DB/Redis wiring gap that only became harmful because of this
|
|
|
|
|
feature**: `vitest.config.ts`'s hardcoded test `DATABASE_URL` (`localhost:5432`) and default
|
|
|
|
|
Redis config don't correspond to any service `docker-compose.test.yml` actually publishes to
|
|
|
|
|
the host, so `test:integration` could never reach a real database under this repo's own
|
|
|
|
|
tooling. This was harmless while every "integration" test was an instantiation-only check (see
|
|
|
|
|
`specs/001-ci-pipeline/checklists/requirements.md`), but this feature's integration test
|
|
|
|
|
(`tests/integration/product-integration-auth.test.ts`) makes real Prisma/Redis calls. Rather
|
|
|
|
|
than leave a newly-introduced test permanently broken for anyone without a coincidentally
|
|
|
|
|
matching local Postgres, fixed `test:unit`'s script to scope to `tests/unit` only (it was
|
|
|
|
|
running the entire `tests/**` glob, including integration/E2E, via no path argument) — matching
|
|
|
|
|
`test:integration`/`test:e2e`'s existing explicit scoping. `test:integration` itself still needs
|
|
|
|
|
a reachable Postgres/Redis (via `docker-compose.test.yml` in CI, or a local equivalent) and was
|
|
|
|
|
manually verified end-to-end against a temporary Docker Postgres/Redis (see PR description) —
|
|
|
|
|
it is not run as part of `npm test`.
|
|
|
|
|
- Manually verified all of spec.md's User Story 1 acceptance scenarios end-to-end against a live
|
|
|
|
|
server + Postgres + Redis (via temporary Docker containers), beyond what the automated tests
|
|
|
|
|
cover: valid in-scope acceptance, indistinguishable invalid-credential/unregistered-product
|
2026-08-21 18:57:07 +05:30
|
|
|
rejection, unknown-field rejection, out-of-scope rejection, and replay rejection.
|
|
|
|
|
- **User Story 2 (admin onboarding/rotation/revocation/audit-trail) is now implemented and
|
|
|
|
|
automatically tested** (`tests/integration/product-integrations-admin.test.ts`, run against a
|
|
|
|
|
real Postgres/Redis, verified passing). **Known limitation carried over from the existing
|
|
|
|
|
codebase, not introduced by this feature**: the admin routes are gated by
|
|
|
|
|
`fastify.authenticate` (`src/plugins/auth.plugin.ts`), which is currently a no-op stub — it
|
|
|
|
|
never actually verifies a JWT or rejects an unauthenticated caller. These admin endpoints are
|
|
|
|
|
therefore not really access-controlled yet. Fixing this requires the `identity/auth` module
|
|
|
|
|
(itself unimplemented) and is out of scope for this feature — flagged here and in
|
|
|
|
|
`contracts/inbound-request-contract.md` so it isn't mistaken for "done."
|
2026-09-02 14:42:20 +05:30
|
|
|
- **User Story 3 (rate limiting) is now implemented and automatically tested**
|
|
|
|
|
(`tests/integration/inbound-rate-limit.test.ts`, run against a real Postgres/Redis, verified
|
|
|
|
|
passing): a bespoke Redis fixed-window counter (`checkRateLimit`,
|
|
|
|
|
`src/infrastructure/cache/rate-limiter.ts`) rather than `@fastify/rate-limit`'s default
|
|
|
|
|
`onRequest`-stage hook — that hook runs before this feature's preHandler-based auth resolves
|
|
|
|
|
the integration/user identity the limit needs to key on, so a second preHandler
|
|
|
|
|
(`checkIntegrationRateLimit`) runs after `authenticateProductIntegration` and checks the
|
|
|
|
|
integration-level limit, then the per-user limit, independently. Verified both are enforced
|
|
|
|
|
independently (a single user's own throttling doesn't affect others; the integration cap
|
|
|
|
|
throttles even when no individual user has hit their own limit).
|
|
|
|
|
- All three user stories (P1, P2, P3) of this feature are now implemented and covered by
|
|
|
|
|
integration tests verified against a live Postgres/Redis, in addition to the unit tests for the
|
|
|
|
|
crypto/token primitives. `docs/06-database-schema.md` itself is intentionally not modified —
|
|
|
|
|
it's the source spec this implementation follows, not generated output.
|