Gemini scanner sweep (token-bypass), GPT-4.1 cross-review, and the
6-detector /sh-security-review fan-out ran against 6acfdab..HEAD; every
confirmed finding fixed:
HIGH (deployment blockers, logic detector):
- CONC-L1: suggestions lambda's bulk line-item PUT sent no
proposalVersion — every AI suggestion job would 422 and be silently
swallowed. Now fetches the proposal's rowVersion, echoes it, and
retries once with a fresh token on 409. Pytest updated (38 green).
- CONC-L2: mobile admin surface (update/approve/send/revise, bulk line
items) sent no tokens — the entire mobile admin workflow would 422.
Tokens threaded through mobile api layer + workspace/line-item
screens with 409 refetch handling. tsc clean.
MEDIUM-adjacent (scanner):
- VendorProposalsController: the VendorTotalCost write on Proposal now
bumps Version (was a silent lost-update path bypassing the guard).
- FilesController: GeneratePDF audit staged into the same SaveChanges.
LOW (detectors):
- 409 envelope is schema-validated client-side
(proposalConcurrencyConflictSchema.safeParse) and id-checked before
seeding the react-query cache; malformed state degrades to
invalidation (INJ-409-01/WEB-CONC-L1).
- ProposalConcurrencyException.CurrentState typed ProposalResponse?
so an EF entity can never serialize into the 409 body (SC-1).
- Guard caller contract documented + GuardedEndpointAuthorizationTests
reflection tripwire: guard-reaching endpoints must stay admin-gated
(AUTHZ-CG-01).
- Pre-check currentState now loads display navigations so both 409
paths return the same shape (CONC-L3).
- Save chain's trailing getById failure no longer misreports a
committed save; falls back to invalidation (CONC-L4).
Also caught during fix verification: the handler's manual currentState
serialization lacked JsonStringEnumConverter — enums would serialize
as numbers, client schema validation would reject every guarded 409,
and the state would always be discarded. Now matches the MVC pipeline
and is pinned by a wire test.
193 xUnit / 70 vitest / 38 pytest green; mobile + shared tsc clean;
Playwright smoke 2/2.
Phase 6b of the SHOC-alignment plan — client side of the optimistic-concurrency
wire contract shipped in 6a.
shared/api-contracts (additive):
- rowVersion: string on ProposalListItem, ProposalDetail, and LineItem
- proposalVersion?: string on UpdateProposalRequest and
BulkUpdateLineItemsRequest; new ProposalVersionRequest
- ConcurrencyConflict<T> { message, currentState } — the SHOC ADR 0004
409 envelope (guarded path; the unguarded fallback carries no state)
- matching zod schemas, all kept under the satisfies z.ZodType<T> coupling
web:
- lib/api/errors.ts: ConflictError carrying the server's reloaded
currentState; client.ts interceptor throws it on 409 (WEB-M2 401
handling untouched)
- guarded mutations read the token from the cached proposal detail at
mutate time; the save flow chains rotated tokens (PUT response token
into the bulk replace) and ends with a detail refetch so
approve-after-save never sends a stale version
- 409 recovery in the admin use-cases: write currentState into the
detail cache, invalidateProposalViews() (stale-queue invariant holds
on the failure path too), and toast the conflict instead of the
generic failure message
- e2e smoke mock payloads carry rowVersion; vitest coverage for the
interceptor ConflictError paths, token threading/rotation, and 409
cache recovery
Verified: shared typecheck, web tsc/vitest (69)/build/prettier/Playwright
smoke, mobile tsc (create-only, no changes needed).
- ProposalConcurrencyTests (shared-connection SQLite, two competing
writers): stale-token pre-check with embedded currentState, DB-level
lost race reloading the winner's values, 422 token codes, version
bump on success, bulk-update proposal-token guard, line-item create
bumping the aggregate, audit rows never persisted on a lost race.
- GlobalExceptionHandlerTests: pin both SHOC 409 envelopes verbatim
(guarded { message, currentState } incl. null state; fallback
{ status, message, code }) and assert shape exclusivity.
- RowVersionCodecTests: round-trip, SHOC-style token literal,
malformed/wrong-length rejection.
- Existing suites threaded with live version tokens (Ver helper);
audit assertions moved from LogAsync to Stage.
187 xUnit green (was 166).
Phase 6a of the SHOC-alignment plan (ADR 0004, wire contract extracted
verbatim from shoc-backend PRs #10/#13-#18).
Concurrency (SHOC double-guard, Postgres port):
- long Version on Proposal + LineItem, IsConcurrencyToken, additive
migration AddProposalLineItemVersion (DEFAULT 1; Up/Down inspected —
no drift, exactly two AddColumn/DropColumn).
- Tokens are opaque base64 strings on the wire (RowVersionCodec:
8-byte big-endian long), rowVersion on responses, proposalVersion on
guarded requests. Missing -> 422 ProposalVersionRequired; malformed
-> 422 InvalidRowVersion (BusinessRuleException carrier).
- Guarded: update, approve, return-to-review, send, revise, and bulk
line-item update (proposal-level token — bulk replaces the item set
wholesale, so per-item tokens are meaningless; deviation from the
plan documented). Creates/deletes unguarded per SHOC precedent but
bump the aggregate version.
- Conflict -> 409 { message, currentState } (SHOC envelope, reloaded
row embedded); bare DbUpdateConcurrencyException -> 409
{ status, message, code } fallback. Both non-ProblemDetails,
emitted by GlobalExceptionHandler.
Audit atomicity (stage-then-single-SaveChanges):
- IAuditService.Stage adds to the shared context without saving;
every proposal/line-item mutation stages before its own single
SaveChangesAsync, so mutation + audit commit or fail together.
LogAsync (self-saving) remains for standalone events (downloads,
role changes, delivery).
Converts the web CI job from ci-typescript-cdk.yaml (typecheck only) to
ci-typescript-frontend.yaml: format:check, build (tsc -b included),
vitest, and a Playwright chromium smoke. Folds the standalone Web Tests
job into it (aggregator needs updated). Pure CI — no AWS secrets.
The smoke (e2e/smoke.spec.ts) drives dev-login → dashboard shell →
proposal list, plus the unauthenticated bounce, against a fully mocked
API (pathname-anchored route interception — a '**/api/**' glob would
swallow vite's /src/lib/api/* module URLs). Config mirrors SHOC's
playwright.config.ts (port 4173, chromium, dev-server webServer).
Prettier: singleQuote + printWidth 100 to match the existing codebase
style; lint intentionally not added (no ESLint config yet — run-lint
false, out of Phase 5 scope). rollback = revert this workflow file.
- AUTH-L1 (confirmed medium): logout() now clears the react-query cache —
the singleton cache survived SPA logout, serving the previous
principal's cached GETs to the next login in the same tab for up to
staleTime with no server round-trip.
- AUTH-L3 (confirmed low): isTokenValid decodes base64url before atob —
valid Cognito JWTs containing '-'/'_' in the payload segment were
misclassified as expired (login lockout/loop; inherited from the old
authSlice).
- AUTH-L2 (unverified, hardened anyway): 401 interceptor broadcasts
AUTH_SESSION_CLEARED_EVENT so AuthProvider drops in-memory state
synchronously, restoring the old Redux atomic-clear semantics.
- INJ-1 (unverified, hardened anyway): Authorization header only set
when the stored token is a string.
Each fix pinned by a test; 63 vitest green, tsc clean.
Correctness:
- State-transition mutations now invalidate every cached view via
invalidateProposalViews (detail + line items + lists + stats + admin
dashboard) — approving no longer leaves a stale queue for the
5-minute staleTime
- Presigned S3 PUT moved to proposals/api.ts with res.ok check — a
rejected upload is no longer confirmed as uploaded
- toCustomerRequest always sends contactEmail ('' clears); API create
path normalizes empty->null to match the update path — customer
emails can now be cleared from the UI
- Shared Number-based numeric form fields (domain/shared/formFields):
'12abc' no longer silently coerces to 12 in the pricing library
- Customer create/update invalidate customersKeys.all so cached search
autocompletes see new customers
- AdminWorkspace clears dirty right after a successful implicit save,
before approve — no false unsaved-changes prompt when approve fails
- ProposalFormPage submit gate and missing-fields caption derive from
ONE checks list (missing customer is now listed)
- Empty states gated on !err in ProposalListPage/AdminDashboard — no
contradictory error + 'no proposals' UI
- VendorDataPanel migrated to useVendorProposals (kills the divergent
['vendorProposals', id] cache key and the inline apiClient query)
- useCustomerList/usePricingLibraryList get keepPreviousData — no
TablePagination out-of-range flash on page change
Cleanup:
- Dead speculative hooks removed (useCreate/BulkUpdate/DeleteLineItem,
useUpdateProposal, useProposalHistory/Audit, lineItemRowFormSchema,
toUpdateLineItemEntry); tests moved to the live save path
(useSaveProposalWorkspace)
- Shared useDebouncedValue hook replaces 4 drifted inline debounce
copies (one leaked its timer on unmount, two hardcoded 300ms);
DEBOUNCE_AUTOCOMPLETE=300 named
- Fix: WEB-H5 / WEB-H6 finding-ID markers restored at the relocated
onError handlers (CLAUDE.md traceability)
- shared/api-contracts gains an exports map; /schemas resolver alias
deduplicated from 3 copies to the tsconfig paths mapping
Verify: tsc clean, vitest 51/51 (tests updated to pin the new
invalidation/mapper behavior + new '12abc' rejection test),
vite build OK, dotnet 166/166.
Additive-only: pages still use lib/api/* and constants/queryKeys.ts until
the page-migration agents run. Each domain ships api.ts (HTTP moved from
lib/api), types.ts (contract re-exports + view types), schemas.ts (contract
schema re-exports + form schemas with toRequest mappers), and use-cases.ts
(TanStack Query v5 hooks + hierarchical query keys, mirroring current page
invalidations and toast-on-error behavior).
Adds an explicit vite/vitest alias for the
@proposal-system/api-contracts/schemas subpath (package has no exports map)
plus a schema/mapper smoke test suite.
Closes WEB-M5 (web hand-duplicated wire types, standing drift risk):
- shared/api-contracts: rewritten as the authoritative superset of the
.NET DTOs (ProposalListItem/ProposalDetail with poNumber and
submittedByName, line item requests, customers, pricing library,
dashboard, audit, sites, auth, presigned upload, ApiProblem); stale
Proposal/UpdateLineItemsRequest shapes removed
- shared/api-contracts/src/schemas.ts: zod runtime schemas coupled to
every wire type via `satisfies z.ZodType<T>` (schema/type drift is now
a compile error); separate entrypoint so type-only consumers (mobile)
never pull zod
- web: imports @proposal-system/api-contracts (file: dep + tsconfig
paths + vite preserveSymlinks); all 7 lib/api modules re-export shared
types so page imports stay stable; enum unions tightened
(PricingLibraryPage form state now ServiceCategory-typed)
- fix(web): customer create/update sent a singular `address` field the
API silently dropped (contract is addresses: string[], CustomerDtos.cs)
- addresses now round-trip, extra addresses preserved on edit
- api: ProblemDetails responses carry a machine-readable top-level
`code` (SHOC error-code vocabulary): ValidationFailed,
InvalidStateTransition, NotFound, Unauthorized, InternalError; new
BusinessRuleException(code, message) maps to 422 with its code;
GlobalExceptionHandlerTests cover the full mapping (wire contract)
Cross-checked .NET DTOs vs TS types vs zod schemas with the
orchestrator scanner (Gemini): core domains consistent; internal-only
DTOs (FileDtos vendor/lambda surface, SimilarProposalDtos, UserDtos
admin surface) intentionally uncovered.
Verify: dotnet 166/166, web tsc + vitest 26/26 + build, mobile tsc,
shared tsc all green.
Moves proof-or-kill-verified false positives (Fastfile runtime PEM-assembly
boilerplate; Podfile.lock CocoaPods SPEC CHECKSUMs) from machine-level to a
tracked repo-local .security-review/suppressions.json so the Open SWE
daily-report automation resolves them. Justifications sanitized to avoid
reproducing the begin-marker literal. Machine-level copy retained until merge.
TypeScript 7.0.2 (the native-port build) no longer exposes the internal
compiler API (ts.sys) that ts-node@10.9.2 depends on, so
`npx ts-node bin/app.ts` fails during `cdk synth` with
"Cannot read properties of undefined (reading 'fileExists')".
Switch the CDK app runner to tsx (esbuild-based, version-agnostic — it
does not consume the typescript package's programmatic API), and pin tsx
as an infra devDependency. Verified `cdk synth` succeeds locally with
typescript 7.0.2 installed.
