diff --git a/.superpowers/sdd/2026-08-01-runtime-correctness-remediation/task-5-report.md b/.superpowers/sdd/2026-08-01-runtime-correctness-remediation/task-5-report.md index 595dc00..1b11417 100644 --- a/.superpowers/sdd/2026-08-01-runtime-correctness-remediation/task-5-report.md +++ b/.superpowers/sdd/2026-08-01-runtime-correctness-remediation/task-5-report.md @@ -81,3 +81,30 @@ The AsyncSurface, catalog, UI test, and type-fixture additions are a narrow scop ## Final review The scoped reviewer completed two fix rounds covering durable ownership, FIFO/races, global bounds, scope fences, and production form settlement. The final verdict reported no findings, independently passed 4 files / 84 tests, confirmed `git diff --check`, and assessed the change ready to merge. + +## Runtime final-review fix round 3 + +The runtime-wide final review identified three additional Task 5 authorities. This round addresses only those findings; the provider-neutral HTTP operation port remains deferred to its separately owned remediation plans. + +### RED evidence + +- Composite optimistic admission: `tests/unit/optimistic-layer-runtime.test.ts` failed 2 / 8 cases because a base-valid candidate that threw only after the prior layer returned a lease and orphaned both rollback and reconciliation authority. +- Candidate replay: the isolated admission test failed with candidate updater call count `2` instead of `1`; replay through `project()` could still delete the entry after successful preflight. +- Form reconciliation: `tests/component/form-foundation.test.tsx` failed 3 / 8 cases. The hook admitted a second submit during unknown effect, settled edited value B instead of submitted snapshot A, and exposed no explicit not-applied release authority. +- Production namespace parity: the mounted reference-page regression failed because `REFERENCE_RESOURCE_QUERY_NAMESPACE` was not exported; production list/detail hooks could only duplicate its id/version literals. + +### Implementation + +1. `OptimisticLayerRuntime.begin()` now computes the complete ordered projection before admission. A composite failure returns pessimistic fallback `null` without changing the existing entry, cache projection, layer IDs, or earlier lease authority. The admitted candidate is written from that precomputed value, so its updater runs exactly once during admission. +2. `useAppForm` retains the exact parsed values for a `MAYBE_APPLIED` submission. The ref is the hook-level admission lock until `settleApplied`, `settleNotApplied`, or `reset` releases it; later edits preserve the unknown result and cannot trigger another command. With `resetOnSuccess: false`, APPLIED makes submitted A the baseline while edited B remains dirty. Success, applied-confirmed, validation/conflict/unavailable outcomes, reset, and explicit not-applied settlement clear the retained snapshot. +3. The reference form routes both reconciliation outcomes into the corresponding form settlement authority. +4. `REFERENCE_RESOURCE_QUERY_NAMESPACE` is exported from the governed feature contract. Both production list and detail query definitions consume its fields, while the mounted-key regression compares both real query prefixes with the installed invalidation edge. + +### Verification + +- Focused runtime/form/reference command: 5 files / 87 tests — PASS. +- `corepack pnpm check:types` — PASS for app, node, test, recipes, web worker, and service worker. +- `corepack pnpm lint` — PASS with zero warnings. +- `corepack pnpm test:all` — PASS: runtime schema 40, unit 744, component 126, integration 23, reference feature 25, recipes 17. +- `git diff --check` — PASS. +- Scoped re-review by the existing Task 5 reviewer: no findings, ready to merge. The reviewer independently passed the 5-file scoped suite (96 / 96), confirmed `git diff --check`, verified all three reconciliation authorities plus candidate single-invocation, and confirmed the deferred HTTP adapter remained untouched. diff --git a/src/features/reference-feature/contracts/reference-feature-contract.ts b/src/features/reference-feature/contracts/reference-feature-contract.ts index 70329f2..3bac006 100644 --- a/src/features/reference-feature/contracts/reference-feature-contract.ts +++ b/src/features/reference-feature/contracts/reference-feature-contract.ts @@ -5,7 +5,7 @@ import { REFERENCE_RUNTIME_SCHEMA_CODECS } from "./reference-schemas.ts"; import { REFERENCE_BOUNDARY_MAPPERS } from "./reference-mapper.ts"; export const REFERENCE_FEATURE_ID = "reference-feature"; -const REFERENCE_RESOURCE_QUERY_NAMESPACE = defineQueryNamespaceIdentity( +export const REFERENCE_RESOURCE_QUERY_NAMESPACE = defineQueryNamespaceIdentity( "reference-resource", 1, ); diff --git a/src/features/reference-feature/presentation/reference-resource-form-page.tsx b/src/features/reference-feature/presentation/reference-resource-form-page.tsx index 368daf7..599bdd6 100644 --- a/src/features/reference-feature/presentation/reference-resource-form-page.tsx +++ b/src/features/reference-feature/presentation/reference-resource-form-page.tsx @@ -131,7 +131,11 @@ export default function ReferenceResourceFormPage() { state={mutation.state} onReconcileUnknownEffect={(resolution) => { void mutation.reconcileUnknownEffect(resolution).then(() => { - if (resolution === "APPLIED") form.settleApplied(); + if (resolution === "APPLIED") { + form.settleApplied(); + } else { + form.settleNotApplied(); + } }); }} /> diff --git a/src/features/reference-feature/presentation/use-reference-feature.ts b/src/features/reference-feature/presentation/use-reference-feature.ts index 3701cf0..64ba9e3 100644 --- a/src/features/reference-feature/presentation/use-reference-feature.ts +++ b/src/features/reference-feature/presentation/use-reference-feature.ts @@ -7,6 +7,7 @@ import { useRouteInput } from "../../../presentation/routes/route-input.tsx"; import { REFERENCE_FEATURE_ID, REFERENCE_RESOURCE_INVALIDATION_TOPIC, + REFERENCE_RESOURCE_QUERY_NAMESPACE, } from "../contracts/reference-feature-contract.ts"; import type { ReferenceCreateCommand, @@ -62,8 +63,9 @@ export function useReferenceDetail(resourceId: string) { definitionId: "reference-resource-detail-v1", definitionVersion: 1, owner: REFERENCE_FEATURE_ID, - namespace: "reference-resource", - namespaceVersion: 1, + namespace: REFERENCE_RESOURCE_QUERY_NAMESPACE.namespaceId, + namespaceVersion: + REFERENCE_RESOURCE_QUERY_NAMESPACE.namespaceVersion, operationId: "GET_REFERENCE_RESOURCE", profileId: "DETAIL_STANDARD", measureResult: measureResourceView, @@ -108,8 +110,9 @@ export function useReferenceFeature() { definitionId: "reference-resource-list-v1", definitionVersion: 1, owner: REFERENCE_FEATURE_ID, - namespace: "reference-resource", - namespaceVersion: 1, + namespace: REFERENCE_RESOURCE_QUERY_NAMESPACE.namespaceId, + namespaceVersion: + REFERENCE_RESOURCE_QUERY_NAMESPACE.namespaceVersion, operationId: "LIST_REFERENCE_RESOURCES", profileId: "LIST_STANDARD", measureResult: measureResourceList, diff --git a/src/presentation/adapters/query/optimistic-layer-runtime.ts b/src/presentation/adapters/query/optimistic-layer-runtime.ts index 38a17a1..45a6ca4 100644 --- a/src/presentation/adapters/query/optimistic-layer-runtime.ts +++ b/src/presentation/adapters/query/optimistic-layer-runtime.ts @@ -38,6 +38,15 @@ export function createOptimisticLayerRuntime(queryClient: QueryClient) { entry.disposeScopeListener = null; } + function writeProjection(entry: EntryState, value: unknown): void { + writing = true; + try { + queryClient.setQueryData(entry.queryKey, value); + } finally { + writing = false; + } + } + queryClient.getQueryCache().subscribe((event) => { if ( writing || @@ -70,12 +79,7 @@ export function createOptimisticLayerRuntime(queryClient: QueryClient) { removeEntry(key, entry); return; } - writing = true; - try { - queryClient.setQueryData(entry.queryKey, value); - } finally { - writing = false; - } + writeProjection(entry, value); } function collapse(key: string, entry: EntryState): void { @@ -132,15 +136,20 @@ export function createOptimisticLayerRuntime(queryClient: QueryClient) { return null; } const layer: Layer = { - id: nextId++, + id: nextId, status: "pending", apply: (value) => update(value, input), }; let projected: unknown; try { - projected = update(entry.base, input); + projected = entry.base; + for (const existingLayer of entry.layers) { + projected = existingLayer.apply(projected); + } + projected = layer.apply(projected); } catch (error) { - if (entry.layers.length === 0) removeEntry(key, entry); + if (entry.layers.length > 0) return null; + removeEntry(key, entry); throw error; } if ( @@ -150,8 +159,9 @@ export function createOptimisticLayerRuntime(queryClient: QueryClient) { if (entry.layers.length === 0) removeEntry(key, entry); return null; } + nextId += 1; entry.layers.push(layer); - project(key, entry); + writeProjection(entry, projected); let state: "pending" | "uncertain" | "settled" = "pending"; const selectedLayer = () => { if (entries.get(key) !== entry) return undefined; diff --git a/src/presentation/forms/use-app-form.ts b/src/presentation/forms/use-app-form.ts index ab11259..663b0cb 100644 --- a/src/presentation/forms/use-app-form.ts +++ b/src/presentation/forms/use-app-form.ts @@ -64,6 +64,7 @@ export function useAppForm< const [pending, setPending] = useState(false); const [result, setResult] = useState("idle"); const pendingRef = useRef> | null>(null); + const unknownSubmissionRef = useRef(null); const dirty = useMemo( () => JSON.stringify(values) !== JSON.stringify(initialValues), @@ -101,7 +102,9 @@ export function useAppForm< return next; }); setFormErrors([]); - setResult("idle"); + setResult((current) => + unknownSubmissionRef.current ? current : "idle", + ); }, [], ); @@ -125,6 +128,7 @@ export function useAppForm< const reset = useCallback( (nextValues: Values = defaultValues) => { + unknownSubmissionRef.current = null; setValues(nextValues); setInitialValues(nextValues); setTouched(new Set()); @@ -137,6 +141,7 @@ export function useAppForm< const settleSuccessfulValues = useCallback( (settledValues: Values) => { + unknownSubmissionRef.current = null; setFieldErrors({} as FieldErrors); setFormErrors([]); setResult("success"); @@ -152,13 +157,24 @@ export function useAppForm< ); const settleApplied = useCallback(() => { - settleSuccessfulValues(values); - }, [settleSuccessfulValues, values]); + const submittedValues = unknownSubmissionRef.current; + if (!submittedValues) return; + settleSuccessfulValues(submittedValues); + }, [settleSuccessfulValues]); + + const settleNotApplied = useCallback(() => { + if (!unknownSubmissionRef.current) return; + unknownSubmissionRef.current = null; + setFieldErrors({} as FieldErrors); + setFormErrors([]); + setResult("idle"); + }, []); const submitForm = useCallback( async (event?: FormEvent): Promise | null> => { event?.preventDefault(); if (pendingRef.current) return pendingRef.current; + if (unknownSubmissionRef.current) return null; setFieldErrors({} as FieldErrors); setFormErrors([]); @@ -194,6 +210,7 @@ export function useAppForm< return outcome; } if (outcome.error.kind === "VALIDATION_REJECTED") { + unknownSubmissionRef.current = null; const mapped = mapValidationFailureToFields( outcome.error, allowedServerFields, @@ -204,16 +221,19 @@ export function useAppForm< setResult("validation-error"); focusFirstError(mapped.fieldErrors); } else if (outcome.error.effect === "MAYBE_APPLIED") { + unknownSubmissionRef.current = parsed.data; setFormErrors([]); setResult("effect-unknown"); } else if (outcome.error.effect === "APPLIED_CONFIRMED") { settleSuccessfulValues(parsed.data); } else if (outcome.error.kind === "CONFLICT") { + unknownSubmissionRef.current = null; setFormErrors([ message("form.conflict"), ]); setResult("conflict"); } else { + unknownSubmissionRef.current = null; setFormErrors([message("form.unavailable")]); setResult("unavailable"); } @@ -251,6 +271,7 @@ export function useAppForm< submitForm, reset, settleApplied, + settleNotApplied, }); } diff --git a/tests/component/form-foundation.test.tsx b/tests/component/form-foundation.test.tsx index 26f03db..7ad8b7e 100644 --- a/tests/component/form-foundation.test.tsx +++ b/tests/component/form-foundation.test.tsx @@ -33,6 +33,7 @@ function FormHarness(props: Readonly<{ | Readonly<{ ok: true; value: string }> | Readonly<{ ok: false; error: ReturnType }> >; + resetOnSuccess?: boolean; }>) { const form = useAppForm({ schema, @@ -45,6 +46,7 @@ function FormHarness(props: Readonly<{ }; }, submit: props.submit, + resetOnSuccess: props.resetOnSuccess, }); return (
void form.submitForm(event)}> @@ -63,6 +65,10 @@ function FormHarness(props: Readonly<{ + + {String(form.dirty)} {form.result}
@@ -152,6 +158,81 @@ describe("local form facade", () => { expect(window.location.href).not.toContain(secretLike); expect(JSON.stringify(localStorage)).not.toContain(secretLike); }); + + it("blocks a second submit while the prior effect remains unknown", async () => { + const user = userEvent.setup(); + const submit = vi.fn(async () => ({ + ok: false as const, + error: createFailure("SERVER_FAILURE", "CREATE_ENTITY", 0, { + effect: "MAYBE_APPLIED", + }), + })); + render(); + const name = screen.getByRole("textbox", { name: /Name/ }); + await user.type(name, "Alpha"); + await user.click(screen.getByRole("button", { name: "Submit" })); + expect(await screen.findByTestId("result")).toHaveTextContent( + "effect-unknown", + ); + + await user.clear(name); + await user.type(name, "Beta"); + await user.click(screen.getByRole("button", { name: "Submit" })); + + expect(submit).toHaveBeenCalledOnce(); + expect(screen.getByTestId("result")).toHaveTextContent("effect-unknown"); + }); + + it("settles the submitted unknown snapshot without accepting later edits", async () => { + const user = userEvent.setup(); + const submit = vi.fn(async () => ({ + ok: false as const, + error: createFailure("SERVER_FAILURE", "CREATE_ENTITY", 0, { + effect: "MAYBE_APPLIED", + }), + })); + render(); + const name = screen.getByRole("textbox", { name: /Name/ }); + await user.type(name, "Alpha"); + await user.click(screen.getByRole("button", { name: "Submit" })); + await screen.findByText("effect-unknown"); + await user.clear(name); + await user.type(name, "Beta"); + + await user.click(screen.getByRole("button", { name: "Confirm applied" })); + + expect(name).toHaveValue("Beta"); + expect(screen.getByTestId("result")).toHaveTextContent("success"); + expect(screen.getByTestId("dirty")).toHaveTextContent("true"); + await user.clear(name); + await user.type(name, "Alpha"); + expect(screen.getByTestId("dirty")).toHaveTextContent("false"); + }); + + it("releases an unknown submission only after explicit not-applied settlement", async () => { + const user = userEvent.setup(); + const submit = vi + .fn() + .mockResolvedValueOnce({ + ok: false as const, + error: createFailure("SERVER_FAILURE", "CREATE_ENTITY", 0, { + effect: "MAYBE_APPLIED", + }), + }) + .mockResolvedValueOnce({ ok: true as const, value: "saved" }); + render(); + await user.type(screen.getByRole("textbox", { name: /Name/ }), "Alpha"); + await user.click(screen.getByRole("button", { name: "Submit" })); + await screen.findByText("effect-unknown"); + + await user.click( + screen.getByRole("button", { name: "Confirm not applied" }), + ); + expect(screen.getByTestId("result")).toHaveTextContent("idle"); + await user.click(screen.getByRole("button", { name: "Submit" })); + + expect(submit).toHaveBeenCalledTimes(2); + }); }); describe("dirty navigation guard", () => { diff --git a/tests/features/reference-feature/reference-page.test.tsx b/tests/features/reference-feature/reference-page.test.tsx index 573e446..f4c7299 100644 --- a/tests/features/reference-feature/reference-page.test.tsx +++ b/tests/features/reference-feature/reference-page.test.tsx @@ -15,7 +15,12 @@ import type { ReferenceFeatureInput, ReferenceResult, } from "../../../src/features/reference-feature/application/reference-feature-api.ts"; -import { REFERENCE_FEATURE_ID } from "../../../src/features/reference-feature/contracts/reference-feature-contract.ts"; +import { + REFERENCE_FEATURE_ID, + REFERENCE_RESOURCE_INVALIDATION_TOPIC, + REFERENCE_RESOURCE_QUERY_NAMESPACE, +} from "../../../src/features/reference-feature/contracts/reference-feature-contract.ts"; +import { INVALIDATION_REGISTRY } from "../../../src/features/installed-feature-contracts.ts"; import type { ReferenceResourceView } from "../../../src/features/reference-feature/contracts/reference-mapper.ts"; import { createFailure } from "../../../src/contracts/errors.ts"; import type { QueryInvalidationCoordinator } from "../../../src/contracts/query-invalidation.ts"; @@ -68,7 +73,7 @@ function renderReference( }); }, }); - return render( + return Object.assign(render( @@ -85,7 +90,7 @@ function renderReference( , - ); + ), { client }); } function inputWith( @@ -114,6 +119,44 @@ function inputWith( } describe("reference feature page states", () => { + it("mounts list and detail keys under the installed governed namespace", async () => { + expect(REFERENCE_RESOURCE_QUERY_NAMESPACE).toEqual({ + namespaceId: "reference-resource", + namespaceVersion: 1, + }); + const installedEdge = INVALIDATION_REGISTRY.edges.find( + (edge) => edge.topicId === REFERENCE_RESOURCE_INVALIDATION_TOPIC, + ); + expect(installedEdge?.namespace).toEqual( + REFERENCE_RESOURCE_QUERY_NAMESPACE, + ); + + const list = renderReference(inputWith()); + await screen.findByRole("heading", { name: "표시할 항목이 없습니다." }); + const listKey = list.client.getQueryCache().getAll()[0]?.queryKey; + expect(listKey?.slice(0, 4)).toEqual([ + "query", + 2, + REFERENCE_RESOURCE_QUERY_NAMESPACE.namespaceId, + REFERENCE_RESOURCE_QUERY_NAMESPACE.namespaceVersion, + ]); + list.unmount(); + + const detail = renderReference( + inputWith(), + "/examples/reference-resources/reference-1", + ); + await screen.findByText("Detail"); + const detailKey = detail.client.getQueryCache().getAll()[0]?.queryKey; + expect(detailKey?.slice(0, 4)).toEqual([ + "query", + 2, + installedEdge?.namespace.namespaceId, + installedEdge?.namespace.namespaceVersion, + ]); + detail.unmount(); + }); + it("renders loading, success and empty states through the installed route", async () => { let resolveList: | ((result: ReferenceResult) => void) diff --git a/tests/unit/optimistic-layer-runtime.test.ts b/tests/unit/optimistic-layer-runtime.test.ts index e3ccbb5..261097a 100644 --- a/tests/unit/optimistic-layer-runtime.test.ts +++ b/tests/unit/optimistic-layer-runtime.test.ts @@ -25,6 +25,82 @@ function scope() { } describe("revision-safe optimistic layer runtime", () => { + it.each(["ROLLBACK", "RECONCILE"] as const)( + "rejects a composite-invalid candidate without losing prior %s authority", + (authority) => { + const client = new QueryClient(); + const key = ["query", "composite-candidate", authority]; + client.setQueryData(key, ["base"]); + const selectedScope = scope(); + const runtime = createOptimisticLayerRuntime(client); + const first = runtime.begin( + key, + "first", + (previous, input) => [...(previous as string[]), input], + selectedScope.snapshot, + ); + if (authority === "RECONCILE") first?.markUncertain(); + + const second = runtime.begin( + key, + "second", + (previous, input) => { + const values = previous as string[]; + if (values.includes("first")) { + throw new Error("candidate composes only over the base"); + } + return [...values, input]; + }, + selectedScope.snapshot, + ); + + expect(second).toBeNull(); + expect(client.getQueryData(key)).toEqual(["base", "first"]); + if (authority === "ROLLBACK") { + first?.rollback(); + } else { + first?.reconcile("NOT_APPLIED"); + } + expect(client.getQueryData(key)).toEqual(["base"]); + }, + ); + + it("writes a prevalidated candidate without invoking its updater twice", () => { + const client = new QueryClient(); + const key = ["query", "single-candidate-invocation"]; + client.setQueryData(key, ["base"]); + const selectedScope = scope(); + const runtime = createOptimisticLayerRuntime(client); + const first = runtime.begin( + key, + "first", + (previous, input) => [...(previous as string[]), input], + selectedScope.snapshot, + ); + let candidateCalls = 0; + + const second = runtime.begin( + key, + "second", + (previous, input) => { + candidateCalls += 1; + if (candidateCalls > 1) { + throw new Error("candidate updater must not be replayed on admission"); + } + return [...(previous as string[]), input]; + }, + selectedScope.snapshot, + ); + + expect(second).not.toBeNull(); + expect(candidateCalls).toBe(1); + expect(client.getQueryData(key)).toEqual(["base", "first", "second"]); + second?.rollback(); + expect(client.getQueryData(key)).toEqual(["base", "first"]); + first?.rollback(); + expect(client.getQueryData(key)).toEqual(["base"]); + }); + it("removes only the failed layer when commands settle out of order", () => { const client = new QueryClient(); const key = ["query", "resources"];