IA audit remediation: decouple publish from the order lifecycle, harden public surfaces, and converge report/repair/agent UX#279
Merged
important-new merged 35 commits intoJul 24, 2026
Conversation
Publishing a report required inspections.status === 'completed', and submitting one for review required the same. Nothing in the product ever wrote that value from the UI, so every publish attempt failed with "Inspection must be completed before publishing the report." while the publish modal reported the report itself as 100% ready — the error named a gate the user had no way to open. The two axes are independent: inspections.status tracks the order (scheduling, dispatch, metrics) and report_status tracks report production. Neither implies the other, so remove the coupling from both paths. Report content completeness is still gated, separately, by publishReadiness — that check is about the report and stays. The two tests that locked the old behaviour now assert the new one: publishing works from every order state and leaves status untouched.
reportActions() opened with `if (inspectionStatus !== COMPLETED) return []`,
so the hub rendered no report actions at all until an inspection was marked
completed — which no surface in the product could do. Every real inspection
landed in the else branch, a single line of text ("Report is still in
progress.") that named no blocker and offered no way forward. The rule it
enforced is gone from the API, so it goes from here too, along with the now
meaningless `inspectionStatus` parameter.
canPublish() encoded the same rule, was covered by assertions in two test
files, and had zero production callers — a gate that was built, tested, and
never installed. Narrow it to the report axis and make isReportShipped its
exact complement, so the predicate has one implementation and that
implementation is the one the page actually renders from.
The remaining branch no longer needs the "any actions?" test to decide
whether to render: the status line always applies, and the action row is
what varies by role. Splitting them also fixes the case that made the old
else branch reachable — a reviewer without publish rights looking at a
submitted report was told it was "still in progress".
Also folds together the duplicates this touched: reportShipped/reportPublished
were two names for one derivation, the blocking-count condition was spelled
out three times, and canPublish/isReportShipped were asserted in both
hub-blocks.test.ts and inspection-hub.test.ts.
Once publishing was decoupled from order completion, the /complete handler
firing report.published became a defect rather than the delivery path: an
inspector who marks fieldwork complete before publishing (now possible, and
common) would trigger report delivery for a report that was never published —
clients would receive a link to an unpublished report. report.published is a
report event; it now fires only from publish. /complete is a pure
order-lifecycle action plus its in-app admin notification.
Also clears the ghost 'delivered' inspection status (IA-74). It is not a
member of INSPECTION_STATUSES, so every `status === 'delivered'` comparison
was dead:
- the idempotency short-circuit's `|| status === 'delivered'` half never
matched — now `=== COMPLETED` via the constant (IA-75's first bare-literal
cleanup);
- getInspectionStatusIcons keyed its `sent` icon solely off 'delivered', so
the icon could never light. The helper had zero production callers and its
test locked in impossible states (`{status:'delivered'} → sent:true`), so
both are deleted;
- inspection-core.service.ts cast status to `'draft'|'completed'|'delivered'`,
a union disjoint from the real enum — now cast to InspectionStatus.
The audit's IA-74 claim of a single occurrence undercounted: the ghost value
had spread to two more sites, exactly as that item predicted.
The order lifecycle's `completed` value had no producer — every other order state was written by some flow (booking, scheduling, concierge, cancel), but nothing marked an inspection completed, so the axis could never advance past confirmed. This adds that producer as an advisory action on the two surfaces an inspector actually uses, never as a gate. Editor (the primary surface — where the inspector finishes on-site): - a "Finish fieldwork" toolbar button, shown until the work is marked complete, POSTing the complete intent; - clicking Publish while not completed opens the existing PublishModal with two paths — "Just publish" and "Mark complete and publish" — both of which publish. Advisory, never blocking, matching the scheduling-conflict philosophy (IA-6). The editor's inspection copy is frozen at mount, so both paths patch status locally (the setInspection pattern structural edits use) to keep the badge live without a reload. Hub: an "Inspection status" card showing the current state and a "Mark fieldwork complete" button while it is still open. Server: a `complete` intent on both the hub and editor actions; the editor's publish intent takes an optional markComplete that fires complete first — idempotent and non-blocking, so it never stops the publish that follows. The Finish-fieldwork button shows from lg up (one step more prominent than the xl-only Sign button, reflecting that it is a more central action). The three touched route files are already grandfathered in the file-size baseline; this bumps their caps for the genuine feature growth.
The publish chain had no end-to-end coverage — `rg "/complete" tests/` was empty — which is precisely how the "publish requires completed, but nothing can mark completed" deadlock reached v1.0.0. These specs pin the decoupling: publishing is offered while the order is still open, and marking fieldwork complete advances the order axis without changing whether publishing is offered. Self-seeds its own template + inspection rather than reusing the shared editor-seed, because it mutates order state (marks complete) and the subsystem-a specs read that seed in parallel. Registered as the lifecycle-publish project, depending on api for the admin workspace.
…s appear
flattenInspectionToRecommendations read each item's field state by the bare
itemId and pulled `.defects` off the top of the entry — but inspection_results
stores entries under the composite findingKey (`_default:{section}:{item}`)
with defects nested under `.tabs`. Both halves were wrong, so defectStates was
always empty and `/agent-recommendations` came back empty for every agent, no
matter how many defects a delivered inspection had. It is an agent account's
only defect view.
Extract the read the report service already did correctly into
readItemEntry / readItemDefectStates (server/lib/read-item-defects.ts) and
route both the report path and the agent path through it, so the key
resolution has one home and cannot drift again.
agent-referral-people.spec.ts was a false green over this exact bug: its
fixture stored `{ item1: { defects: [...] } }` — the bare-key, no-tabs shape
the UI never writes — which only the broken reader could parse. Corrected to
the canonical storage shape, so it now guards the real path.
Fixing the read unmasks IA-41 (custom defects and tenant-defined categories
are still dropped by the cannedById / category guards) — tracked separately.
…ity roll-up
buildSystemsSummary rolls up each system's worst condition from the item
`severityBucket`, but the report pipeline only ever writes the getRatingBucket
domain (`satisfactory | monitor | defect | other`). asSeverity accepted the
OTHER axis (`marginal | significant | minor`), so every real bucket fell
through to 'good' and worstSeverity was always 'good' — the commercial PCA
summary printed "Good" for a system even while the same row counted four
safety defects. Map the buckets to the severity axis (the inverse of
getRatingBucket) so the roll-up reflects what was actually rated. This feeds
both the web report and the DOCX export (via pca-report-block →
report-export-consumer); there is no duplicate asSeverity to change.
Two report specs fed the fix its own blind spot: pca-systems-summary.spec.ts
and pca-report-block.spec.ts both set severityBucket to severity-axis values
('marginal'/'significant'/'good') the pipeline never emits, so the always-good
bug stayed green. Both now feed the bucket domain.
IA-43 tracks unifying the two domains so this local inverse can be retired.
…ayload getReportData spread the whole inspections row into data.inspection, and GET /api/public/report/:tenant/:id returns that data verbatim to any link holder — including agent-kind tokens. The row carries internal_notes (the inspector's private notes) and price_cents (commercial pricing), which the account track already withholds from agents at the schema layer (agent.schema.ts). The public track only avoided leaking them because the SSR page happens not to render them — page-doesn't-render is not a boundary. Delete both from the object at the point it becomes the public payload. The internal `inspection` row keeps every column for getReportData's own logic, so nothing downstream of the read changes. The audit defers severity and any broader projection to a dedicated security review; this is the boundary it names.
resolveBuilderAccess Path 1 assigned creator.kind='client' to any resolvable portal token, so an agent-kind token (buyer_agent / listing_agent) acted as the client on the repair builder's five write endpoints, and an other-kind token (attorney, title company, …) acted as one too. The sibling client-actor resolver already refuses non-client kinds; Path 1 did not. Resolve the grant's role kind via a new PortalAccessService.getRoleKind (role KEY → contact_role_profiles.kind, so tenant-custom profiles are honored, not just the seed keys): client/co_client → client, agent → agent, anything else → no builder actor (rejected). An agent now reaches the builder as an agent, and the two agent paths (portal token vs agent-portal session) resolve to the same kind — asserted directly so they cannot drift apart again. This is IA-73's unconditional half (stop the identity impersonation). The tenant-level agentRepairAccess switch (off/read/readwrite) that gates whether agents may act at all lands next, on top of this.
Completes IA-35: after the identity fix labeled agents correctly, this lets a tenant decide whether agents may act on the repair request list at all — `off` (no access), `read` (view only), `readwrite` (full, the default). Default readwrite: account-track agents already receive a builder link (dashboard), so defaulting off would remove a shipped ability. The setting lives in the inspectionPrefs JSON (no migration) and is read by InspectionCoreService.getAgentRepairAccess. resolveBuilderAccess now carries an accessLevel and consumes the switch in every agent branch — portal token (Path 1), legacy KV token (Path 2), and agent session (Path 4) — so the two agent tracks can never diverge. The shared write guard (runAssertCanEdit) and the create-list handler refuse a read-only actor with 403; client and inspector actors are always readwrite. Settings → Inspection exposes the three levels as a RadioGroup (matching the sibling controls on that page rather than the plan's RadioCardGroup, for visual consistency). Tests cover all three states plus a direct assertion that the token track and the session track resolve to the same action set.
…ish (IA-36) revokeForRecipient and setExpiryForInspection were implemented and tested but had no API caller, so a delivered report link stayed live forever and unpublishing retracted the report while leaving its links working. Wire both cascades at the route layer (services stay decoupled): - Removing a person from an inspection now revokes that recipient's report-access token. removePerson returns the removed email so the handler can revoke; revokedAt already beats any expiry in both resolution paths, so the link is dead regardless of the report-link TTL. - Unpublishing expires every access token for the inspection immediately — no live URL should point at a withdrawn report. The People-card Remove button gains a confirmation (no window.confirm) that names the side effect — "Their report link stops working immediately" — and is relabeled "Remove from inspection", because a silent access revocation on an already-ambiguous button is exactly what the audit flagged. This is the security core of IA-36 (the registry's "most-worth-fixing half": existing capability, only the call sites + the confirming UI were missing). The fuller lifecycle — Reset/rotate, Make-primary, per-recipient status, a tenant report-link TTL, an expiry landing page — remains as follow-up.
…rywhere CommandPalette (and its global Cmd/Ctrl+K listener) was mounted only on /inspections, so the shortcut did nothing on /calendar, /settings/*, or anywhere else in the authenticated workspace — the product's strongest discoverability surface existed on one page (IA-49). Hoist the single mount to auth-layout, the workspace shell that wraps every non-fullscreen page. The own-chrome routes (the hub, the editors) sit outside auth-layout and keep their own key handling.
The sidebar's search button carried an "open command palette" aria-label but had no onClick, so its most visible entry point did nothing — a dead control is worse for trust than no control. Lift the palette's open state to auth-layout, expose it through a CommandPaletteContext, and point the button at it. CommandPalette becomes controlled (open / onOpenChange); its Cmd/Ctrl+K listener reads the current open through a ref so the once-registered handler still toggles correctly. Only IA-49's hoist to auth-layout made this reachable from every page in the first place.
…late (IA-39) The Install button had no onClick — importTemplate (a full backend implementation behind POST /api/templates/marketplace/:id/import) was never reachable from the UI, so clicking Install did nothing. Add a BFF resource route (never a raw client fetch) that forwards to the import endpoint, and point the button at it via useFetcher: the pressed card shows an "Installing…" pending state, a failure surfaces a danger banner, and success jumps to the freshly imported template in the library. The route action is unit-tested for success / failure / missing-id.
…-40)
The signed, immutable report-version system was fully built server-side but
had no way in: the version-diff page had zero inbound links (only reachable by
hand-typing the URL), the publish form never sent the `summary` the API
already accepted, and — discovered during E2E — the diff page itself crashed on
every real diff.
- inspection-hub: the Report card now lists report versions (number, published
date, Amendment badge, change summary), each amendment linking to a
field-level diff against its predecessor via versionDiffHref(). The loader
fetches the version list best-effort.
- publish flow: an amendment (versionNumber > 1) now shows a "what changed"
textarea in the publish modal and forwards it as `summary` to
snapshotOnPublish, which already accepted it.
- version-diff page: the loader/component expected a flat DiffEntry[] but the
API returns { items, units }, so `data.map` threw on any non-empty diff — the
page only ever "worked" when there were zero changes. flattenVersionDiff()
now maps the real payload into the rendered rows (changed fields show
before/after; added/removed items and units render a marker).
Registry premise about the data layer being fully wired was wrong on the last
point; corrected per code-over-docs. Verified in Chrome (light + dark).
The rating axis carried two value domains with no single bridge, and the same value wore contradictory labels across documents. - Label: the PCA report printed severity `minor` as "Minor" (reads as a small issue) while the editor prints it "N/A" (not applicable). `minor` maps to the getRatingBucket 'other' bucket and, via getNaKind, represents Not Inspected / Not Present — i.e. not-applicable, not a minor defect. Renamed pca_severity_minor to "Not Applicable" so the two documents no longer contradict each other. good/marginal/significant keep their per-document wording (residential inspection vs commercial ASTM E2018 PCA are different audiences — permitted). - Domain bridge: added bucketToSeverity() to report-utils as the single, explicit inverse of getRatingBucket, with a round-trip test. pca-systems- summary now imports it and drops the private copy it had carried since IA-32. No behavior change in the summary roll-up; the shared inverse is identical to the retired local one.
…-42) The repair builder's "Severity" sort never sorted by severity — SEVERITY_RANK was keyed on `category` (safety/recommendation/maintenance), so a defect-rated item marked 'maintenance' sank below a satisfactory item marked 'safety'. The constant name, the sort-key string, and the button label all said "severity" while reading the category axis — the product's most misleading name. - Renamed the category ranker CATEGORY_RANK and gave it its own "Category" sort option; added a real SEVERITY_RANK over the rating-axis bucket (defect worst, not-applicable last). - Plumbed the parent item's severityBucket through getRepairList → flattenReportDefects → the repair-builder source API → the builder's Defect, so the new "Severity" sort ranks by the actual rating. - "Severity" now genuinely means the rating axis; "Category" names the other. Library's Repair Item form keeps "Severity" = rating axis (unchanged). Sort orderings covered by unit tests. Browser walk of the three sort buttons is data-heavy (needs a published inspection with categorized, severity-varied defects — no such fixture locally) so it is deferred; the value-level sort tests plus the compiled end-to-end plumbing stand in.
…(M-A: IA-54/60/62/68) Terminology cleanup across the repair and report surfaces: - IA-54: the Library page contradicted itself — "Repair Items" title over copy that said "repair recommendation". Both now say "repair item". - IA-60: the repair builder's category pill (Safety/Recommendation/Maintenance) was hardcoded English on a client-facing surface; extracted to paraglide (portal_repair_category_*), resolved at call time for the locale scope. - IA-62: the inspector-added marker read "custom" in the editor badge but "inspector-added" in the report — same isCustom bit, two words. Unified the editor badge to "inspector-added". (Per the settled decision, residential "Section" vs PCA "System" stay distinct — two correct industry vocabularies, not an inconsistency.) - IA-68: the report's "View Repair List" button pointed at /inspections/:id/repair-list, a page route that never existed (only the API route does), so it 404'd. Removed the dead button — "Build repair request" already reaches the real capability. The e2e "page sub-route is mounted" test accepted 404 as a pass (false-green for a phantom route); removed it, keeping the real API-route assertion. Not done: IA-54's item-level recommendationId badge (the doc's rename target no longer exists in the code). IA-54's agent route/file/API rename lands next.
…A-54) Third of the three agent-side names for one thing. The sidebar label and page heading already said "Repair Items"; the route URL, the file, and the API path still said "recommendations". - Route /agent-recommendations → /agent-repair-items (old path 301s to the new one, mirroring recommendations-redirect.tsx). - File routes/agent/recommendations.tsx → repair-items.tsx. - API GET /api/agent/my-recommendations → /my-repair-items (+ operationId). - Regenerated the OpenAPI snapshot to match. The agent-recommendations *service* (business logic) keeps its name — only the user-reachable route/URL/API converge on "Repair Items". Message-key names are left as-is (their rendered values already read "Repair Items").
…gories (IA-41) The agent Repair Items feed was the only one of three consumers that silently dropped two kinds of finding — and with IA-31 fixed, the page went from "always empty" to "missing the most important rows", which would read as a new bug. - Field-added custom defects (customComments.defects) were never read: the flatten only walked the template's tabs.defects and continued on any miss. Added a custom-defect pass (mirroring the report + repair-list surfaces), tagging those rows isCustom for an "inspector-added" badge. - Defects tagged with a tenant custom category were dropped: isCategory gated to the three legacy buckets and both the flatten and the grouping discarded anything else. The category is now kept verbatim, and grouping merges non-safety/-maintenance (recommendation + any custom category) into the Recommendations bucket — the same merge direction as the PCA Systems Summary, so the two client surfaces agree instead of dropping vs mis-bucketing. - The page shows an "inspector-added" badge on custom rows and a note when custom-category findings were merged, so nothing vanishes without a trace. Merge chosen over per-category dynamic groups: contical with the PCA summary and far more contained, per the registry's stated fallback. Sort/group behavior is unit-tested; the agent page's browser walk needs an agent account with referred, published inspections carrying custom defects (no such fixture locally), so it is deferred to the value-level tests + compiled plumbing.
…ing (IA-47) An agent viewing a report via their token link who tapped "email me a sign-in link" landed on /agent-dashboard after clicking the emailed link — dropped off the property they were looking at, having to hunt it back down. - Thread a returnTo through the whole magic-login flow: the BFF action derives it server-side (/portal/:tenant/i/:id?token=§ion=report — never from the client, so it can't be an open-redirect vector), the request stores it in the single-use KV payload, and the redeem step redirects there instead of the dashboard. - safeReturnTo() guards the redeem redirect (a session cookie is minted immediately before it): only a same-origin relative path is honoured; scheme-relative //, backslash tricks, and absolute URLs fall back to the dashboard. Scope note (code-over-docs): the registry's second half assumed signup does not mint a session, but agent-signup already sets __Host-inspector_token, so that sub-item is already satisfied. The remaining ask — lifting the token-track read-only restrictions (hideClientActions / forced section=report) for a logged-in agent on the report page — is the deliberate IA-35 security boundary and is not loosened here; an agent with a session uses /agent-repair-items (now populated by IA-41) as their workspace.
…fect categories (M-C: IA-66/59) - IA-66: the report's "Defects Only" filter used a severityBucket regex that dropped 'monitor' while the same component's "Add to repair request" checkbox showed for defect OR monitor. Both now read the tenant's per-category drivesSummary switch via itemDrivesSummary(). Also fixes a latent bug: the regex never matched a tenant custom category, silently excluding them. - IA-59: the field custom-defect dropdown offered only the three built-in categories; it now lists the tenant's defect_categories (threaded from the editor loader) alongside the built-in seeds. CustomDefectCategory widened to a defect_categories.id/legacy-name string. M-C's heavier three (IA-55 + migration, IA-56, IA-57) land in the continuation.
…+ estimate hint (M-C: IA-55/56) The repair list is the buyer↔seller negotiation + contractor-quote document; it was dropping the three fields that make it actionable. - IA-55: the public /repair-request/:shareToken page showed only Section / Item / Finding(comment) / Note / Credit — two defects on one item were indistinguishable, a contractor got no location, and the category the client sorted by was dropped on the last hop. Added defect title + location + category as SNAPSHOT columns on repair_request_items (migration 0001, appended at table end), captured at add time so the shared list stays stable after the report changes. Plumbed title/location through getRepairList → flattenReportDefects → the source API → the builder Defect → the add-item body → the snapshot → the share page (Finding cell now leads with the title + location; a Priority column shows the category). - IA-56: the report's Est. Cost never reached the credit builder, so clients hit a blank Credit Request. RepairDefectRow now shows "Report estimate: $x–$y" with a "Use estimate" button that fills the field (dollars→cents) — it never writes a value or auto-submits, and the estimate is not snapshotted to the share page (RRB stays pure-credit). Migration is 3 plain ADD COLUMNs (no rebuild); db:check clean.
Two prior commits left correct, needed changes in the working tree: - IA-54 (agent-recommendations -> agent-repair-items rename, b6db657) committed the renamed page + messages but not the route wiring: routes.ts (new path + 301 redirect from the old one), agent-layout sidebar link, the agent.ts API path (/my-recommendations -> /my-repair-items, already reflected in the committed OpenAPI snapshot), and the redirect route file. HEAD was left with the page calling a path the router did not expose. - IA-55 (repair share snapshots, ebe7ca3) committed migration 0001_gifted_doorman.sql but not its drizzle meta (_journal.json entry + 0001_snapshot.json), leaving migrations/ and meta/ inconsistent. No behavior change beyond making the tree match what those commits intended; db:check is green with the meta in place.
…-57 render half)
The editor captures a per-defect Trade and Timeframe (DefectFieldsRow), but
they only reached the client via Mustache interpolation into the comment
prose. A template author who left out the {{trade}}/{{timeframe}} placeholder
dropped them silently -- the inspector filled fields no reader could see
(audit S2.3.3).
Resolve them onto the report payload and give them their own render point:
- inspection-report.service: compute the defect Mustache vars once and reuse
them, exposing effectiveTrade / effectiveTimeframe (same label resolution as
the comment path, so a stored slug renders identically in both places).
- ResolvedDefect: carry effectiveTrade / effectiveTimeframe (null when blank).
- ReportDefectCard: a muted meta row under the comment, "Recommended trade:"
and "Timeframe:", each shown only when set. Tokens only (text-ih-fg-4),
dark-mode-safe by reuse of the adjacent location line style.
- messages/en/reports: two new labels (paraglide, no hardcoded strings).
Deferred (logged in the registry as IA-57 partial):
- Repair-request public page: rendering trade/timeframe there needs snapshot
columns + the full builder->share plumbing (a second migration mirroring
IA-55); out of scope for the no-migration render half.
- Vocabulary unification (contractor_types as the single authority for the
Trade dropdown): reconciling three taxonomies would break the live
{{trade}} label pipeline and needs a provisioning seed change. Its
recommendation-categories.ts deletion is a no-op -- that catalog is still
consumed by the per-defect recommendationId feature.
…ew (H-B4: IA-45) The standalone /report-gate page had no producer (nothing in app/ links to it) and its own "Sign agreement" CTA could fall back to itself: when no signer link could be reconstructed, getReportGateContext set actionUrl to /report-gate/..., the same page rendering it -- a closed loop (audit S4.6.4). Because it's an exception path off the happy path, nobody filed a bug. Absorb its job -- "why is my report locked, and what do I do" -- into the Hub overview the client already reaches, and retire the route: - InspectionStatusCards: reportLockNotice(ov) -- pure, agreement-before-payment precedence (matching the server gate), returns null once nothing is outstanding (a not-yet-published report with no gate is pending on the inspector, not client-actionable). - InspectionHub overview: renders the notice inline above the status cards with a CTA Link to ?section=agreement / ?section=payment (same route, so no "cannot rebuild signer link" problem). DS tokens only (warn + primary), dark-safe by reuse of the existing status-card warn tone. - report-gate route: retired to a 301 -> /portal/:tenant/i/:id?section=overview (fossil redirect, mirrors /reports and /recommendations). - inspection-publish.service: the no-signer-link fallback now points at the Hub overview instead of the self-referential /report-gate URL. Note: the /api/public/report-gate endpoint is kept as a harmless fossil (still tested); no code path renders its payload now that the page redirects. TDD: reportLockNotice matrix (4 cases) + report-gate 301 redirect test. Chrome E2E deferred -- the overview lock banner needs a gated inspection behind a portal session, and local dev has zero published/gated inspections plus degraded portal flows; acceptance rests on the pure-function tests + grep (no producer, no self-reference) + type-check.
…(H-B4: IA-46)
OI's e-sign has two halves -- "signs" and "can be independently verified
later" -- but the second half wasn't being delivered. The permanent
/verify/:envelopeId page (signer list + content snapshot + Ed25519 audit chain)
was only reachable by hand-pasting an envelope id. Close the three remaining
delivery gaps (the premise was already narrowed 2026-07-23: the signed-
confirmation email DOES carry the link, so this is delivery-coverage, not
"no link is ever built"):
- Gap 1 -- workflow fallback pointed at JSON, not the page. The sign-completion
workflow's evidence-pack email fell back to `${base}/api/public/verify/:id`
(a JSON API surface that renders no UI) whenever the verification token wasn't
yet persisted. Route the fallback to the human /verify/:envelopeId page.
- Gap 2 -- remote co-signers got nothing. The in-person sign route already emails
every signer their own receipt; the REMOTE route (bookings/agreement.ts) only
ran the envelope-completion pipeline, which emails the envelope client alone.
A remote co_client / agent signer walked away with no verify link at all. Add
the same per-signer receipt with the same completed-self dedup guard, so each
remote signer gets the tamper-evident link at their own address.
- Gap 3 -- the Hub agreement section had no verify entry. Surface the
/verify/:envelopeId link in the signed branch of <AgreementSection>. Needed a
small additive server change after all (code over docs: the GET /agreements/
:token response did not carry the envelope id) -- add `envelopeId` to the
response + schema and thread it through AgreementData.
Also collapses the verify-URL construction (previously inlined in sign-effects
and the workflow) into a single envelopeVerifyUrl() helper, so the page-vs-JSON
distinction can't drift again.
TDD: envelopeVerifyUrl matrix (page path, base join, relative fallback);
AgreementSection render (link present when signed, absent pre-sign / no id,
never the /api surface); remote-sign spec now asserts each signer's receipt +
verify URL. Chrome E2E deferred -- the signed-agreement Hub surface needs a
signed envelope behind a portal session and local dev has zero signing data
plus degraded portal flows; acceptance rests on the render tests, the server
specs, and grep (no /api/public/verify fallback, receipt call present, link
threaded).
…okens (H-B4: IA-37) OI put lifecycle controls on the tokens that EXCHANGE for a session (magic-link TTL, observer-link revoke, concierge single-use) but not on the tokens that CARRY content -- signer links (agreement_signers) and repair-request share links (repair_requests) were permanent and un-revocable. A two-year-old signing email still opened checkout; a forwarded /repair-request/:shareToken (with defect list + requested credit -- a negotiation lever) worked forever. Give both tokens the same three-part lifecycle the rest of the product already has: - Schema: add nullable expiresAt / revokedAt to agreement_signers and repair_requests, appended at the table end so D1 does a plain ADD COLUMN rather than a referenced-table rebuild (reference_d1_add_column_at_end). NULL expiresAt = never expires (legacy / synthesized / backfilled rows); revocation always wins over expiry. - Enforcement (fail closed): getSignerByPresentedToken and RepairRequestService.getByShareToken drop a revoked-or-expired row to null, so a dead link resolves to NotFound at the route rather than to a signable envelope / readable share page. Predicate + TTL constants live in one server/lib/token-ttl.ts so issuance and enforcement can't drift. - Issuance: findOrCreate stamps signer links with a 90-day TTL; RepairRequest create stamps share links with 180 days (contractors forward those longer). The tenant-configurable override (reportLinkTtl) and the friendly "this link expired -- ask for a new one" landing page are the IA-36 lifecycle work; until they land these constants are the only source and a dead link fails closed to NotFound (safe enforcement, just not yet a branded explanation). Migration 0002 is a pure 4x ADD COLUMN (drizzle meta committed alongside). TDD: token-ttl predicate matrix; signer-token-lifecycle (issued TTL, live resolves, revoked/expired fail closed); share-token-lifecycle (same). Pure server-side, no user-visible surface -- no Chrome E2E this round.
…apability bits (M-B: IA-51, IA-53) The agent portal promised more than it wired up, on two fronts. IA-51 — /agent-dashboard grouped referrals by inspection COMPANY, but an agent's unit of work is the deal, not the vendor. An agent arriving from a report email (context = one property) had to first recall which company inspected it before finding that property inside a company section. Regroup by property/transaction: the section header is now the normalized address (two referrals at the same address from different companies collapse into one group of two rows), the company name is demoted to inline row metadata beside the inspector, groups sort by most-recent inspection date, and the just-converted ?welcome referral's group is pinned first. Company becomes a SECONDARY filter (a dropdown, shown only when the agent partners with more than one company). /agent-inspectors is unchanged — that page's object really is the company. IA-53 — capabilitiesForKind returned canSign/canPay for client and agent, but NOTHING in the codebase ever read them (only selfRetrieveReport is consumed). A never-read capability reads to the next person as "supported, just not wired up" and invites accidental wiring. Delete both bits: agent self-sign / self-pay is a product decision we are not making, and if revisited it needs its own model (co-signer attestation, payment/report-access coupling), not a dormant boolean. Report access is unaffected — it flows through the token/account tracks and selfRetrieveReport, not these bits. TDD: dashboard.test.tsx gains same-address-different-company merge, address header / inline company, secondary-filter, and single-company-hides-filter cases (existing welcome/timezone cases still green); capabilities.spec.ts asserts the trimmed shape and that canSign/canPay are gone. Chrome E2E for the agent dashboard is data-blocked locally (zero agent/referral data), so acceptance rests on the render tests + type-check + grep.
…nge-history read (M-D: IA-50, IA-63, IA-64) Three workspace-shell gaps where a capability existed but had no reachable surface. IA-50 — the command palette truncated every group to 8 items, so 6 of the 14 Settings destinations never rendered when the palette was browsed without a filter word (they were reachable ONLY by already knowing the keyword). Remove the blanket per-group cap: every source group is already bounded (Pages / Settings are fixed lists; Recents is now sliced to RECENTS_CAP at its source), so nothing needs re-truncating. Also add the two settings destinations that had no palette entry at all — Email Templates (/settings/communication/templates) and QuickBooks (/settings/integrations/qbo). (The parametric /settings/communication/templates/:trigger cannot be a static entry.) The palette was already promoted to auth-layout by IA-49, so this now applies workspace-wide. IA-63 — /metrics only had workspace totals; a multi-inspector company needs per-inspector count / revenue / turnaround for team management and commission. Add a byInspector aggregate grouped by lead_inspector_id (falling back to inspector_id — the schema's stated authority for "who did this inspection"), joined to users for the name, with average turnaround = first publish (report_versions v1 published_at) minus inspection date in days. The LEFT JOIN keeps unpublished inspections in the count while avg() skips their NULL turnaround, so an all-unpublished inspector reports null, not 0. New "By Inspector" table on the page. IA-64 — template.* / comment.* audit rows were being WRITTEN but never read back, so "who changed the Roof wording, and when" had no answer in the UI. Add a tenant-scoped GET /api/audit/entity/:entityId (newest first, actor name resolved, owner/manager only), a BFF resource route, and a reusable <EntityAuditTrail> disclosure that lazy-loads on open (no N+1 on the parent list) and shows "Last edited by X" plus the full history. Wired into the Comments list and the Templates list view. The always-inline attribution the audit sketched is realized as an on-demand disclosure — it cures the same invisibility without a per-row fan-out. New API surface → openapi snapshot regenerated; audit route added to api-types + the per-module hono client. server/index.ts crossed the file-size baseline by two lines (one import + one mount) — baseline updated. TDD: metrics-by-inspector.spec (lead-wins attribution, revenue, null-vs-0 turnaround); entity-audit-trail.spec (newest-first, tenant isolation, limit); CommandPalette.test (all settings render when browsing, new entries reachable by keyword); EntityAuditTrail.test (collapsed by default, reveals attribution, empty state). The metrics table and the "Last edited by" line are data-blocked locally (zero published inspections / no seeded audit rows), so those rest on the unit + render tests; the palette is data-independent and verified live.
…-E: IA-61)
A re-published report notified the client with the exact same "report ready"
mail as the first publish — no way to tell a fresh report from an amendment,
and no carrier for the change summary the inspector already types on re-publish.
The data layer knew the difference (report_versions.isAmendment = version > 1);
the notification did not.
Split the report event into two triggers:
- report.published — the first publish (unchanged).
- report.amended — any re-publish (a prior report_versions row exists). Resolved
in publishInspection via resolvePublishTrigger BEFORE the new snapshot is
written, so "a prior row exists" == "this publish produces version >= 2".
The amended trigger gets its own seeded templates (client + buyer's agent, in
BOTH seed paths — standalone.ts rows and AUTOMATION_SEEDS) framed as an update
and carrying {{summary}} — the inspector's change note, newly plumbed as a
template variable sourced from the latest report_versions row (fetched once per
inspection per flush, gated to amended emails only so the common triggers pay
nothing). The delivery path's report.published PDF/token branch intentionally
does not claim amended, so amended mail flows through the generic template path
with the new template.
Surfaced end to end: the trigger enum + its Zod mirror gain report.amended, the
automations settings screen gets a "Report amended" label, and the publish
modal's notify toggle reads "Notify client of this amendment" when re-publishing
(the modal already received isAmendment from the version-diff work).
New enum value → openapi snapshot regenerated; enum is type-layer only (no DDL /
migration). inspection-publish.service.ts crossed the file-size baseline by four
lines — baseline updated.
TDD: report-amended-trigger.spec (first publish -> report.published; a prior
version -> report.amended; tenant + inspection scoped) and an automation-seeds
addition (the report.amended client rule seeds active; its seed body carries
{{summary}}). The delivery/summary wiring is data-blocked locally (no published
inspection to flush), so it rests on the unit + type checks; the publish-modal
wording is data-blocked for E2E and rests on the render path + isAmendment tests.
… IA-67) /observe/inspections/:id rendered live per-section progress behind an observer-link token — the most complete credential lifecycle in the product, and a second entry to a capability the client portal already owns (the Hub's "progress" section). It was also the ONLY entry with no way to reach it: no email template builds the link, the mint/list/revoke API has no app-side caller, and nothing in app/ links to /observe. A built surface with zero inbound paths. Retire the surface: observe.tsx now renders a static retirement notice pointing the visitor to their client portal, instead of fetching + rendering ProgressView. The observer-link service, schema, cookie middleware, claim handler, and the token-gated observe API are all left intact — that token machinery is the reference implementation to reuse when the portal's own credential lifecycle is built, so it should not be torn out with the page. Not a redirect into the portal: the observer-link token is a separate credential system from the portal's magic-link session and this route carries no tenant slug, so a 301 would only drop the visitor on a login wall their token can't pass. A notice that names where progress now lives is the honest retirement. Pure app change — no server, schema, migration, or API-surface edits (the observe endpoint stays for the preserved backend). The two now-unused error strings are removed; two retirement strings added. TDD: observe.test.tsx (renders the retirement notice pointing to the client portal; no live section-progress UI). Verified live in Chrome (light + dark) — the route serves the notice, not a 404 or the old ProgressView.
…symbol keys (N: IA-75)
The inspection and report axes each declare a single source of truth
(server/lib/status/{inspection,report}-status.ts) whose header requires every
consumer to derive from the exported constants — "no bare status string
literals". Nothing enforced it, so member values were hand-typed in several
places. A hand-typed value is exactly how a ghost value reaches runtime: written
as a literal, it never meets the type layer that would reject it, and no reviewer
catches it unless they happen to look. The repo already treats "a rule that only
lives in a comment is a latent bug" as a house principle — lint:migrefs,
lint:timestamps, lint:tz all make a discipline executable. This does the same for
the status axes.
New gate (lint:status-literals, wired into `npm run lint` beside its sibling
lint:tenant-scope): scan server/ and app/ for a member value written as a bare
literal bound to a status key — an assignment (status: 'completed',
.set({ status: 'confirmed' })) or a comparison (status === 'completed'). Using
the derived constant passes. The member words are ordinary English reused by
unrelated axes (an erasure job's status, an outbox row's delivery status), so the
detector over-includes by design; the baseline-ratchet model freezes the current
16 hits and fails only on NEW ones, with --update-after-review as the escape
hatch for a genuinely different axis. A union-type guard skips type declarations
(status: 'a' | 'b'), which are not writes.
Baseline key scheme, shared by both gates via scripts/lib/symbol-baseline.mjs:
`relpath::symbol::signature`. The older tenant-scoping baseline was line-keyed,
which made every edit above a frozen hit renumber it into a false failure that
forced a mass --update — noise that masked real new hits. The symbol anchors a
hit to its enclosing function instead of a line, so unrelated edits no longer
move it; the signature (a normalized snippet of the matched code) keeps two hits
in one function distinct, so a NEW hit added to an already-baselined function is
still caught rather than collapsing into the existing key. tenant-scoping is
converted to the same scheme in this change so a second gate does not grow a
second baseline mechanism with the same line-keyed pain; its detector
(findUnscopedByIdQueries) is unchanged and only gained an `index` field for symbol
resolution, so its unit test is untouched and still green. The regenerated
tenant baseline drops from 95 to 74 entries: identical-context hits in one
function (genuine duplicates) collapse to one key, while distinct queries keep
distinct signatures — no loss of coverage.
The demonstrative publish.ts literals were already converted to
INSPECTION_STATUS.COMPLETED earlier; the remaining existing hits (concierge,
event, google-calendar's external axis, outbox) are frozen in the baseline, not
mass-rewritten — that cleanup is out of scope for adding the gate.
TDD: symbol-baseline.spec.ts (enclosing-symbol resolution incl. class methods,
scope-opening vs plain locals, module fallback; makeKey/diff granularity) and
status-literal-gate.spec.ts (assignment/comparison detection, constant passes,
non-status-key literal ignored, object-key ignored, union-type ignored, comment
skipped). Both gates smoke-verified end to end: each fails exit 1 on a freshly
introduced hit and returns green once removed.
…d heatmap card (N: IA-80)
The /metrics page and its API had drifted apart at the field level. The endpoint
returns the monthly series as `data.monthly[]` with `{ month, count, revenue }`,
but the page's interface and both month charts read `data.months[].ym` — a name
that never existed in the response. Result: the "Inspections per Month" and
"Revenue per Month" charts rendered their empty state for every tenant even when
the server returned real data. Because a hand-written fixture could be shaped to
either side, no test caught it; the only fix that stays honest is to bind the
page to the server's actual response shape.
- MetricsData + both month-chart renderers now read `data.monthly` / `mo.month`.
A comment on the interface pins the field names to server/api/metrics.ts so the
two ends can't silently diverge again.
- The "Findings Heatmap" card read `data.heatmap`, which the endpoint does not
return at all — there is no heatmap aggregation in the metrics handler, so the
card was permanently empty. A dormant AnalyticsService.findingsHeatmap() exists
but is unwired and emits long-form `{ section, category, count }` cells over six
rating buckets, not the card's wide-form three columns; connecting it is a
category-mapping design decision, not a field rename. The honest fix here is to
remove the dead card (and its six now-unused message keys) rather than leave a
visualization that can never populate. Wiring that aggregation is logged as a
follow-up.
TDD: metrics.test.tsx renders MetricsPage via createRoutesStub with the server's
response shape and asserts the month bars render (counts + `MM` labels) instead of
the empty state, plus the empty state when the series is absent. Live-data Chrome
E2E is deferred: the bars require seeded multi-month inspection data the local DB
does not have, so a zero-data page would only show empty states — the unit test,
feeding the exact server shape, is the stronger check for a field-name binding.
Pure app change — no server, schema, or API-surface edits. type-check:app 0,
test:web green, eslint 0 errors, lint:ds/i18n/i18n-catalog/tests/filesize OK.
Booking is company-level: `/book/:tenant` and `/embed/:tenant` are the public entries, and the server auto-assigns the first available qualified inspector. Two legacy per-inspector URL forms lingered as a second shape for the same capability — the exact "one capability, two URLs" ambiguity that costs a reader a lookup every time to conclude the two are identical and one is history. `inspector-signature.ts` already collapsed inspector-shared "Book again" links to the company page; this finishes the job for the remaining surfaces. Removed: - `book/:tenant/:slug` route + booking-inspector-redirect.tsx (it only 302'd to `/book/:tenant?inspector=<slug>`; the `?inspector=` query form on the company page still works directly, so nothing is lost). - `embed/:tenant/:slug` route. - `bookingUrl()` and `embedBookingUrl()` from public-urls.ts — both build a per-inspector path and had no production caller (only their own unit cases). The embed part was NOT a straight delete: booking-embed.tsx also housed the shared EmbedWizard/EmbedData that the company embed (booking-embed-company.tsx) imports. Deleting it would have broken the company embed. So it is renamed to booking-embed-widget.tsx — a plain shared component with the per-inspector route's loader, page, meta, and route-type import stripped out — and the company route imports from there. The widget's rendering logic is unchanged (moved, not rewritten). CommandPalette's "Copy booking link" quick action built the removed `/book/<tenant>/<slug>` path; deleting the route without touching it would leave the action copying a dead link. It now offers the company URL `/book/<tenant>`, matching the precedent inspector-signature.ts already set. The `currentUserSlug` still gates the action to bookable inspectors; the slug is just no longer in the URL. Kept intact (out of this change's scope): `users.slug` (audit attribution + the `/api/public/book/:tenant/:slug` profile endpoint), and the company page's `?inspector=<slug>` deep-link resolution + that backend endpoint — the query form and its backend are a separate surface, retired here is only the redundant path/URL-builder forms. The user.ts slug comment is updated to describe the endpoint's real reachability (via `?inspector=`) now that the redirect is gone. TDD: CommandPalette.test.tsx asserts the copy action offers the company URL, not the per-inspector path (renders with an inspector session context). public- urls.spec.ts drops the two deleted builders and re-points the localhost-scheme case onto embedBookingCompanyUrl. No Hono API route changed — mcp:snapshot shows no drift. type-check api+app 0, eslint 0 errors, lint:ds/tests/filesize/ status-literals OK. Live Chrome E2E deferred: the booking/embed/palette surfaces need a running dev server + a seeded tenant (and the palette action needs an authenticated inspector session); the widget move is logic-identical and fully covered by type-check + the unit tests above.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remediates a full information-architecture audit of the inspection workflow: report delivery was coupled to the order lifecycle, several data fields were authored but never surfaced downstream, a handful of pages and URL forms were orphaned or duplicated, and some content-bearing public links never expired. This is one milestone bundling those fixes, grouped by theme below.
Publish / order-lifecycle decoupling
report.publishedas a side effect.Public surfaces & token security
internal_notesand price are kept out of the public report payload./verifylink is delivered to every signer and surfaced in the Hub.Agent portal & repair items
Reports & editor
Workspace UX
Automation
Retirements & consistency
Tooling / lint gates
PCA & marketplace
All changes ship with unit/component coverage; the full suite (type-check, lint,
db:check,test:unit,test:web, build) is green.🤖 Generated with Claude Code