282 lines
15 KiB
Markdown
282 lines
15 KiB
Markdown
# Runtime Correctness Remediation Implementation Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Make topic invalidation hit every real bound query and make each logical mutation preserve a unique intent and effect-aware optimistic state.
|
|
|
|
**Architecture:** Contracts own query-key and mutation-intent shapes. Bootstrap indexes feature invalidation contributions once. Presentation creates one intent per admitted logical submit, while HTTP consumes that intent and optimistic settlement follows the returned effect certainty.
|
|
|
|
**Tech Stack:** TypeScript 7, React 19, TanStack Query 5, Zod 4, Vitest 4.
|
|
|
|
## Global Constraints
|
|
|
|
- Query persistence remains disabled; the key-version change has no persisted migration.
|
|
- Query keys and invalidation prefixes are created only by `src/contracts/query-keys.ts`.
|
|
- Cross-context messages carry topics, never namespace IDs, query keys, input identities, intent IDs, or idempotency keys.
|
|
- A logical mutation creates one intent after duplicate admission and reuses it for every physical attempt.
|
|
- A missing `KEYED` idempotency key fails before credentials, fetch admission, or diagnostics containing caller data.
|
|
- `MAYBE_APPLIED` never rolls back, commits, invalidates, or retries automatically.
|
|
- Every production behavior change is preceded by a focused failing test.
|
|
|
|
---
|
|
|
|
### Task 1: Query-key V2 and namespace identities
|
|
|
|
**Files:**
|
|
- Modify: `src/contracts/query-keys.ts`
|
|
- Modify: `src/contracts/server-state.ts`
|
|
- Modify: `tests/component/application-query.test.tsx`
|
|
- Modify: `tests/unit/query-invalidation-registry.test.ts`
|
|
|
|
**Interfaces:**
|
|
- Adds `QUERY_KEY_SCHEMA_VERSION = 2`.
|
|
- Adds `QueryNamespaceIdentity = { namespaceId: string; namespaceVersion: number }`.
|
|
- Adds `defineQueryNamespaceIdentity`, `createBoundQueryKey`, `createQueryInvalidationPrefix`, and `queryNamespaceIdentityKey`.
|
|
- Changes `bindQuery` to delegate key construction to `createBoundQueryKey`.
|
|
|
|
- [ ] **Step 1: Add failing key/prefix parity tests**
|
|
|
|
```ts
|
|
const namespace = defineQueryNamespaceIdentity("reference-resource", 1);
|
|
const bound = bindQuery(definition, input, scope);
|
|
expect(bound.queryKey).toEqual([
|
|
"query", 2, "reference-resource", 1,
|
|
scope.fingerprint, definition.definitionVersion, bound.identity.token,
|
|
]);
|
|
expect(bound.queryKey.slice(0, 4)).toEqual(
|
|
createQueryInvalidationPrefix(namespace),
|
|
);
|
|
```
|
|
|
|
Also reject empty/control-character IDs, non-positive versions, and excessive UTF-8 length.
|
|
|
|
- [ ] **Step 2: Run RED**
|
|
|
|
Run: `corepack pnpm exec vitest run tests/component/application-query.test.tsx tests/unit/query-invalidation-registry.test.ts`
|
|
|
|
Expected: missing helper exports and current V1 key order mismatch.
|
|
|
|
- [ ] **Step 3: Implement the shared constructors**
|
|
|
|
`createBoundQueryKey` must return exactly:
|
|
|
|
```ts
|
|
Object.freeze([
|
|
"query", QUERY_KEY_SCHEMA_VERSION,
|
|
namespace.namespaceId, namespace.namespaceVersion,
|
|
scopeFingerprint, definitionVersion, identityToken,
|
|
]);
|
|
```
|
|
|
|
`createQueryInvalidationPrefix` returns the first four entries. `bindQuery` constructs the namespace identity from the definition rather than duplicating the tuple.
|
|
|
|
- [ ] **Step 4: Run GREEN**
|
|
|
|
Run: `corepack pnpm exec vitest run tests/component/application-query.test.tsx tests/unit/query-invalidation-registry.test.ts`
|
|
|
|
- [ ] **Step 5: Commit**
|
|
|
|
```bash
|
|
git add src/contracts/query-keys.ts src/contracts/server-state.ts tests/component/application-query.test.tsx tests/unit/query-invalidation-registry.test.ts
|
|
git commit -m "fix: align bound query keys with invalidation prefixes"
|
|
```
|
|
|
|
### Task 2: Many-to-many invalidation in production composition
|
|
|
|
**Files:**
|
|
- Modify: `src/contracts/query-invalidation.ts`
|
|
- Modify: `src/features/reference-feature/contracts/reference-feature-contract.ts`
|
|
- Modify: `src/features/installed-feature-contracts.ts`
|
|
- Modify: `src/adapters/query-cache/tanstack-cache-coordinator.ts`
|
|
- Modify: `src/bootstrap/runtime-adapters.ts`
|
|
- Modify: `tests/unit/query-invalidation-registry.test.ts`
|
|
- Modify: `tests/unit/tanstack-cache-coordinator.test.ts`
|
|
- Modify: `tests/features/reference-feature/reference-contract.test.ts`
|
|
- Modify: `tests/unit/runtime-adapters.test.ts`
|
|
|
|
**Interfaces:**
|
|
- `InvalidationRegistry.namespaces` and edges use `QueryNamespaceIdentity`.
|
|
- `indexInvalidationRegistry` returns every namespace identity for each topic.
|
|
- `createTanStackCacheCoordinator` consumes `InvalidationRegistryIndex`; topic versions remain a separate bounded map used only by cross-context transport.
|
|
- Installed features export `INVALIDATION_REGISTRY`, composed once at bootstrap.
|
|
|
|
- [ ] **Step 1: Add failing real-key invalidation and fan-out tests**
|
|
|
|
Seed `QueryClient` with real `bindQuery(...).queryKey` values, map one topic to two namespaces, call local and remote invalidation, and assert both matching queries are invalidated while an unrelated namespace is not. Assert the published event contains only topic/version.
|
|
|
|
- [ ] **Step 2: Run RED**
|
|
|
|
Run: `corepack pnpm exec vitest run tests/unit/query-invalidation-registry.test.ts tests/unit/tanstack-cache-coordinator.test.ts tests/features/reference-feature/reference-contract.test.ts tests/unit/runtime-adapters.test.ts`
|
|
|
|
Expected: the coordinator accepts the legacy flat registry and invalidates prefixes that do not match bound keys.
|
|
|
|
- [ ] **Step 3: Compose and index contributions once**
|
|
|
|
Feature contribution shape:
|
|
|
|
```ts
|
|
invalidation: Object.freeze({
|
|
topics: [REFERENCE_RESOURCE_INVALIDATION_TOPIC],
|
|
namespaces: [defineQueryNamespaceIdentity("reference-resource", 1)],
|
|
edges: [{
|
|
topicId: REFERENCE_RESOURCE_INVALIDATION_TOPIC,
|
|
namespace: defineQueryNamespaceIdentity("reference-resource", 1),
|
|
}],
|
|
})
|
|
```
|
|
|
|
`installed-feature-contracts.ts` concatenates these bounded contributions. `runtime-adapters.ts` calls `indexInvalidationRegistry(INVALIDATION_REGISTRY)` exactly once, derives transport topic versions, and passes the index to each generation's coordinator.
|
|
|
|
- [ ] **Step 4: Make the coordinator invalidate every indexed prefix**
|
|
|
|
For each topic, iterate `namespacesForTopic.get(topic)`, create the V2 prefix with `createQueryInvalidationPrefix`, and call `invalidateQueries({ exact: false, refetchType: "active" })`. Sequence-gap handling visits all indexed topics without duplicating namespace work.
|
|
|
|
- [ ] **Step 5: Remove legacy authorities**
|
|
|
|
Delete `QUERY_REGISTRY` from `src/contracts/query-keys.ts`, the flat installed `QUERY_REGISTRY`, `InstalledQueryInvalidationDefinition`, and feature-owned concrete TanStack namespace tuples after `rg` shows zero callers.
|
|
|
|
- [ ] **Step 6: Run GREEN**
|
|
|
|
Run: `corepack pnpm exec vitest run tests/unit/query-invalidation-registry.test.ts tests/unit/tanstack-cache-coordinator.test.ts tests/features/reference-feature/reference-contract.test.ts tests/unit/runtime-adapters.test.ts`
|
|
|
|
- [ ] **Step 7: Commit**
|
|
|
|
```bash
|
|
git add src/contracts/query-invalidation.ts src/features/reference-feature/contracts/reference-feature-contract.ts src/features/installed-feature-contracts.ts src/adapters/query-cache/tanstack-cache-coordinator.ts src/bootstrap/runtime-adapters.ts tests/unit/query-invalidation-registry.test.ts tests/unit/tanstack-cache-coordinator.test.ts tests/features/reference-feature/reference-contract.test.ts tests/unit/runtime-adapters.test.ts
|
|
git commit -m "fix: index many-to-many query invalidation"
|
|
```
|
|
|
|
### Task 3: Application-owned mutation intent
|
|
|
|
**Files:**
|
|
- Create: `src/contracts/mutation-intent.ts`
|
|
- Create: `src/application/ports/mutation-intent-factory.ts`
|
|
- Create: `src/adapters/platform/browser-mutation-intent-factory.ts`
|
|
- Modify: `src/contracts/server-state.ts`
|
|
- Modify: `src/presentation/adapters/query/application-query.ts`
|
|
- Create: `src/presentation/adapters/query/mutation-intent-provider.tsx`
|
|
- Modify: `src/presentation/adapters/query/server-state-generation-provider.tsx`
|
|
- Modify: `src/bootstrap/runtime-adapters.ts`
|
|
- Modify: `src/features/reference-feature/adapters/reference-http-gateway.ts`
|
|
- Modify: `src/adapters/http/http-effect-certainty.ts`
|
|
- Modify: `src/adapters/http/http-execution-v3.ts`
|
|
- Modify: `tests/component/application-query.test.tsx`
|
|
- Modify: `tests/unit/runtime-adapters.test.ts`
|
|
- Modify: `tests/unit/http-execution-v3.test.ts`
|
|
- Modify: `tests/integration/http-execution-contract.test.ts`
|
|
|
|
**Interfaces:**
|
|
- `MutationIntent` has the exact approved immutable shape.
|
|
- `MutationIntentFactory.create({ operationId, canonicalInputIdentity, requiresIdempotencyKey })` returns one intent.
|
|
- `BoundMutation.execute` context adds `intent: MutationIntent`.
|
|
- `HttpExecutionContext.intent` consumes the application intent without regenerating it.
|
|
|
|
- [ ] **Step 1: Add failing lifecycle tests**
|
|
|
|
Assert two independent submits receive different intent/key pairs; a `JOIN_IDENTICAL` waiter shares the admitted submit; physical HTTP retry sees the same key; queries have no intent header; diagnostics and URLs contain neither intent ID nor key.
|
|
|
|
- [ ] **Step 2: Run RED**
|
|
|
|
Run: `corepack pnpm exec vitest run tests/component/application-query.test.tsx tests/unit/runtime-adapters.test.ts tests/unit/http-execution-v3.test.ts tests/integration/http-execution-contract.test.ts`
|
|
|
|
Expected: bound mutation context has no intent and bootstrap produces resettable `http-key-N` values.
|
|
|
|
- [ ] **Step 3: Define intent validation and browser factory**
|
|
|
|
Validate bounded non-empty strings and finite non-negative monotonic timestamps. Use `crypto.randomUUID()` independently for `intentId` and required idempotency key; permit deterministic injected factories in tests.
|
|
|
|
- [ ] **Step 4: Create intent after duplicate admission**
|
|
|
|
Keep canonical identity calculation before duplicate lookup. Only the execution that wins admission calls the factory. Pass the same frozen intent through `mutation.mutateAsync({ input, intent })` and every bound mutation/feature gateway call.
|
|
|
|
- [ ] **Step 5: Remove adapter-local sequence identity**
|
|
|
|
Delete `contractExecutionSequence`, `http-intent-N`, `http-key-N`, and the unused HTTP-layer `MutationIntent` factory. Bootstrap passes the supplied intent into `ContractHttpExecutor` unchanged.
|
|
|
|
- [ ] **Step 6: Run GREEN**
|
|
|
|
Run the command from Step 2 and expect all intent lifecycle assertions to pass.
|
|
|
|
- [ ] **Step 7: Commit**
|
|
|
|
```bash
|
|
git add src/contracts/mutation-intent.ts src/application/ports/mutation-intent-factory.ts src/adapters/platform/browser-mutation-intent-factory.ts src/contracts/server-state.ts src/presentation/adapters/query/application-query.ts src/presentation/adapters/query/mutation-intent-provider.tsx src/presentation/adapters/query/server-state-generation-provider.tsx src/bootstrap/runtime-adapters.ts src/features/reference-feature/adapters/reference-http-gateway.ts src/adapters/http/http-effect-certainty.ts src/adapters/http/http-execution-v3.ts tests/component/application-query.test.tsx tests/unit/runtime-adapters.test.ts tests/unit/http-execution-v3.test.ts tests/integration/http-execution-contract.test.ts
|
|
git commit -m "fix: preserve logical mutation intent"
|
|
```
|
|
|
|
### Task 4: Fail KEYED commands before dispatch
|
|
|
|
**Files:**
|
|
- Modify: `src/adapters/http/http-execution-v3.ts`
|
|
- Modify: `tests/unit/http-execution-v3.test.ts`
|
|
- Modify: `tests/integration/http-execution-contract.test.ts`
|
|
|
|
- [ ] Add tests for absent, empty, control-character, and over-budget keys. Spy on `attachCredentials` and `fetch`; both must remain at zero and the result must be `CONTRACT_VIOLATION` with `effect: "NOT_STARTED"`.
|
|
- [ ] Run `corepack pnpm exec vitest run tests/unit/http-execution-v3.test.ts tests/integration/http-execution-contract.test.ts` and confirm RED because KEYED commands currently dispatch without a key.
|
|
- [ ] Add `MISSING_IDEMPOTENCY_KEY` to the request violation union and validate before credential resolution. Reject a key on `NONE`/query descriptors as the same pre-dispatch contract class.
|
|
- [ ] Re-run the focused tests and confirm GREEN, including same-key physical retry.
|
|
- [ ] Commit with `git commit -m "fix: reject invalid keyed mutation intents"`.
|
|
|
|
### Task 5: Effect-aware optimistic settlement and reconciliation
|
|
|
|
**Files:**
|
|
- Modify: `src/presentation/adapters/query/optimistic-layer-runtime.ts`
|
|
- Modify: `src/presentation/adapters/query/application-query.ts`
|
|
- Modify: `src/application/view-models/async-state.ts`
|
|
- Modify: `src/contracts/errors.ts`
|
|
- Modify: `tests/unit/optimistic-layer-runtime.test.ts`
|
|
- Modify: `tests/component/application-query.test.tsx`
|
|
|
|
**Interfaces:**
|
|
- `OptimisticLayerLease` adds `markUncertain()` and `reconcile("APPLIED" | "NOT_APPLIED")`.
|
|
- Layer status becomes `pending | uncertain | committed`; collapse stops before unresolved uncertain layers.
|
|
- Controller adds `reconcileUnknownEffect(resolution)` tied to the original intent.
|
|
- Async state adds `mutation-effect-unknown`.
|
|
|
|
- [ ] **Step 1: Add failing certainty matrix tests**
|
|
|
|
Cover `NOT_STARTED`, `NOT_APPLIED`, `APPLIED_CONFIRMED`, and `MAYBE_APPLIED`; out-of-order later commits; applied/not-applied reconciliation; scope closure. Assert unknown effect does not call `invalidate` or expose generic retry.
|
|
|
|
- [ ] **Step 2: Run RED**
|
|
|
|
Run: `corepack pnpm exec vitest run tests/unit/optimistic-layer-runtime.test.ts tests/component/application-query.test.tsx`
|
|
|
|
Expected: current catch path rolls every failure back.
|
|
|
|
- [ ] **Step 3: Derive settlement before touching optimistic state**
|
|
|
|
Use `failure.effect ?? "NOT_STARTED"` only for failures known to be pre-dispatch. The mutation bridge switches explicitly:
|
|
|
|
```ts
|
|
switch (effect) {
|
|
case "NOT_STARTED":
|
|
case "NOT_APPLIED": rollback(); break;
|
|
case "APPLIED_CONFIRMED": commit(); scheduleInvalidation(); break;
|
|
case "MAYBE_APPLIED": markUncertain(); exposeReconciliation(); break;
|
|
}
|
|
```
|
|
|
|
- [ ] **Step 4: Preserve ordered uncertain layers**
|
|
|
|
Projection still applies uncertain layers. `collapse` may consume committed layers only until the first pending/uncertain layer. Reconciliation converts uncertain to committed or removes it, then reprojects all later layers.
|
|
|
|
- [ ] **Step 5: Run GREEN**
|
|
|
|
Run the command from Step 2 and expect all certainty and ordering cases to pass.
|
|
|
|
- [ ] **Step 6: Commit**
|
|
|
|
```bash
|
|
git add src/presentation/adapters/query/optimistic-layer-runtime.ts src/presentation/adapters/query/application-query.ts src/application/view-models/async-state.ts src/contracts/errors.ts tests/unit/optimistic-layer-runtime.test.ts tests/component/application-query.test.tsx
|
|
git commit -m "fix: retain uncertain optimistic mutations"
|
|
```
|
|
|
|
### Task 6: Runtime correctness verification
|
|
|
|
- [ ] Run `corepack pnpm exec vitest run tests/unit/query-invalidation-registry.test.ts tests/unit/tanstack-cache-coordinator.test.ts tests/unit/http-execution-v3.test.ts tests/unit/optimistic-layer-runtime.test.ts tests/component/application-query.test.tsx tests/integration/http-execution-contract.test.ts tests/features/reference-feature/reference-contract.test.ts tests/unit/runtime-adapters.test.ts`.
|
|
- [ ] Run `corepack pnpm check:types`.
|
|
- [ ] Run `corepack pnpm lint`.
|
|
- [ ] Run `corepack pnpm test:all`.
|
|
- [ ] Run `git diff --check`.
|
|
- [ ] Record any browser-only gate as unverified unless its Playwright command actually ran.
|