feat: deliver private PWA with records, evidence and discovery - #1
Merged
Merged
Conversation
…n plan Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…legacy production seed Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…old membership Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… tests Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…or tests Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…imeouts, v2/options import) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… directly Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Functions emulator spawns its runtime worker lazily on the first request; on this Windows-mounted path (/mnt/c) that worker's cold require() of functions/lib + firebase-admin measured 73.9s via a direct curl call, exceeding the 60000ms testTimeout on its own and explaining why the first two whoami.test.ts tests timed out in fix round 1. Add warmUpFunctions() to emulator-helpers.ts, which pings the callable and retries on connection errors until any response arrives. Call it first in whoami.test.ts's beforeAll, with an explicit 300000ms hook timeout, so the worker is warm before any per-test 60000ms budget is spent. testTimeout and assertions are unchanged. Verified: two consecutive plain `npm run emu:test` runs, no ad-hoc overrides, both 18 passed (171.77s, 172.82s). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… ruling) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…name warmUpFunctions previously retried on connection errors forever; a vitest hook timeout doesn't cancel that in-flight loop, it only fails the test while the loop keeps running. Give it its own bound via maxElapsedMs (default 240000ms), throwing once elapsed time exceeds it instead of retrying past it. Also drop the "whoami" default on name so a future test can't silently warm the wrong function. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…overs) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ngine Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… modern event types Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ount guard Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…al setup Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ount) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…om Plan 1 final review Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…config guard; gitignore scope; CI guardrail Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rror for per-request timeouts (Plan 3 pre-work) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… cross-realm-safe abort/timeout predicates Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…usage-day key Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…vider with magic queries Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd 8 s timeout; provider selection with the deployed fixture refusal Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…il-closed kill switch, per-day cap counted before the provider call, provider error mapping, privacy-safe logs Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…onfig, deploy-package exclusions; callable and rules emulator tests; secret-absent probe Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…mbered search controller with per-request timeout and offline short-circuit Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ry failure state, results with Google Maps attribution and links, In our records matching Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…-state prefill, place id on create, duplicate guard, Open in Google Maps on the record Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XSp7ebmUaqxSfwhK9nNXud
…ll switch, cap, provider failures, timeout, supersede, Near me granted/denied, add to records, offline); README Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nded provider log messages with a log-content test
- functions/src/discovery/callables.ts: pass { region: "europe-west2", maxInstances: 2 } explicitly on
both onCall definitions instead of relying on setGlobalOptions() in index.ts running first — onCall
snapshots global options eagerly at definition time, so the cost guardrail (maxInstances: 2) must not
depend on import order.
- functions/test/discovery.endpoint.test.ts: imports searchDestination/searchNearby directly from
../src/discovery/callables (not index) and asserts the real __endpoint shape found by probing it
empirically: region as ["europe-west2"], maxInstances 2, secretEnvironmentVariables
[{ key: "PLACES_API_KEY" }].
- functions/src/discovery/search.ts: cap the logged provider/unexpected-error message to 200 chars —
logs must never carry unbounded caller-supplied content (spec §2.6, §3.6).
- functions/test/discovery.search.test.ts: mocks firebase-functions/logger and asserts over every
logged call that a destination query's text never appears, nearby coordinates appear only rounded to
2dp, and a provider message that echoes the query is truncated to at most 200 chars.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rail greps
- web/e2e/emulator-rest.ts: getRestaurant now decodes doubleValue (a decimal lat/lng was silently
dropped, so scenario 10's not.toHaveProperty("lat")/("lng") could never fail) and keeps any other
Firestore value type as its raw object instead of dropping it, so every stored field stays visible
to assertions.
- .github/workflows/ci.yml: raise job timeout-minutes from 25 to 35 (the browser suite grew to 23
scenarios, globalTimeout is now 900s, scenario 6 alone waits ~25s); add a guardrail step — "discovery
never caches Places data or carries a key" — that fails on browser-storage use under web/src/discover,
an AIza-shaped literal under web/src (excluding the config validator), or a non-fixture
PLACES_API_KEY= value committed anywhere outside *.md (AGENTS.md documents an unrelated legacy
GOOGLE_PLACES_API_KEY var whose name contains that substring).
- functions/vitest.config.mts: retry once under CI only — the Firestore emulator has been observed to
flake under back-to-back runs ("Request time should not be before the last token refill time"); local
runs keep retries off so a flake stays visible.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…mes, deferred minors, final review and fix wave Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XSp7ebmUaqxSfwhK9nNXud
A Near me request that resolves after a newer text/Near me search or after the page unmounts could still supersede the newer result or fire a paid searchNearby call. Track a search-intent generation and discard any late position/failure whose intent no longer matches (audit F1). Adds unit regressions for supersession, unmount, and a late location failure, plus two Playwright scenarios (12, 13) against the emulator. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A bare textQuery search was an unrestricted place lookup — a town name like "Lisbon" returned localities/addresses, not just restaurants, and the mapper kept any result with no businessStatus regardless of type. Text Search now sends includedType: "restaurant" with strictTypeFiltering: true, and the field mask adds places.types so the mapper can independently guarantee every returned result's `types` includes "restaurant" — a server-side check that holds even if Google's own filtering doesn't exclude something (audit F3). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… logger
The installed firebase-functions/logger overwrites a structured
payload's `message` field with the positional string ("discovery.search")
before writing — so the bounded provider/unexpected-error message this
code set was silently discarded, and the old "≤200 chars" tests only
proved that about a mock, never about what actually gets logged.
search.ts now logs fixed diagnostics only (outcome/status for a
ProviderError, outcome/errorName otherwise) and never provider- or
error-controlled text. The test suite drops the firebase-functions/logger
mock and exercises the real logger: console.info/warn/error are patched
inside vi.hoisted() (before any import, since the logger's write()
captures a console.* reference once, at its own first load, which a
later vi.spyOn cannot intercept), and assertions run against that real
captured output.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… findings §3.6 overstated the free tier (10,000 → the documented 5,000 requests per SKU per month, aggregated across the billing account) and called the committed fixture a "recorded real response" when it is a synthetic one in the documented shape. Also documents the F3 restaurant-only search restriction, the fixed-diagnostics logging change, and opens a new "owner decision" subsection recording audit S1: the user-saved storage exception ruling 1 relies on could not be substantiated against Google's current terms, so ruling 1 and the prefill flow stay on hold for an owner decision. README's discovery guardrail bullet is updated to match (types in the mask; restaurant-restricted text search). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner ruling on audit S1/F2: Google's name and address are shown in Discover results but never stored or prefilled; a record created from a result holds the place id (permitted indefinitely) and a name and address the member types. RestaurantPrefill drops name/address, the create draft always starts empty, and the prefill notice links by place id alone (placeIdUrl) instead of by name. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Spec §3.6 ruling 1 now states the ruling directly (place id only, the member types name and address) instead of the invalid "user-saved exception"; the "Open owner decision" subsection Fix D added is replaced by the ruling. README's discovery guardrail bullet and the Plan 3 header note the same. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…og Google's error status Controller ruling (Fix F, post-review of audit fixes A-E): strict restaurant-only filtering hid dedicated gluten-free bakeries and cafés. Drop strictTypeFiltering on Text Search (keep includedType as a bias), send the full VENUE_TYPES list on Nearby Search, and let the mapper keep any place typed restaurant, cafe, bakery, bar or meal_takeaway. Also thread Google's error status enum (fixed allow-list) through ProviderError into the discovery.search log payload, for operability before staging, without ever logging free-text error content. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Update the design spec §3.6 and README for the controller ruling (cafés/bakeries/bars/takeaways alongside restaurants), the dropped strictTypeFiltering, the fixed Google error-status allow-list in logs, the current npm run test:unit count, and the empty-draft place-id notice wording already shipped for the record form. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…, owner ruling on S1/F2, venue-type ruling, gate) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XSp7ebmUaqxSfwhK9nNXud
Reset confirmations when claims change, show mixed-snapshot offline notices, disable pending deletion offline, and reject malformed URL spellings before writes. Add component and emulator-browser regressions.
Record the owner-approved search modes, keep one provider call per submit, and preserve raw queries from legacy clients that omit mode. Cover all accepted venue types and mode validation, propagation, and compatibility.
Archive independent findings and diagnostic probes, record their fixes and fresh local gate, and document the approved search-mode decision and legacy-client compatibility review.
Bogdan0708
changed the title
Bogdan0708
marked this pull request as ready for review
September 23, 2026 14:58
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
Summary
Delivers the private SafeBite PWA through Plans 1–3: invited household members can sign in, keep restaurant records and dated evidence, and discover food venues through authenticated, quota-limited functions. Non-members are refused. The legacy Swift application and tests remain unchanged.
Validation
Fresh local gate on the final code: typechecks and builds passed; 270 web unit tests, 282 functions/rules emulator tests and 29 emulator browser tests passed. Browser retries were disabled. Boot guard / preview / upgrade suites passed 1 / 4 / 7 tests. Permanent regressions cover live claim replacement, cached snapshot combinations, malformed URLs, mode propagation and legacy-client request compatibility.
Hosted CI run 35831695682 passed every web-and-functions step on head
5be710004f4a24d2e5eb65223c7afde28f79893din 4m42s. GitGuardian also passed on that head.Independent review and reproduction evidence are committed under
planning/audits/, including2026-09-23-landing-review.md. Earlier reports retain their historical findings; the landing review records the fixes and the owner-approved product decision. The old foundation-only audit follow-ups have been addressed.Release boundary
This merges reviewed application code; it does not deploy Firebase or establish staging/device acceptance. O4 (restricted pilot Places key), real-provider search relevance and real-iPhone/Safari acceptance remain outstanding. Deploy functions before hosting for the search contract, and coordinate rules with hosting for record writes. No real credentials, billing settings or provider data were introduced.
Plan 4 will start in a fresh worktree from the merged main branch.