Skip to content

fix(firebase): declare the composite indexes cortex's queries need [TURNWRK-775] - #45

Merged
practical-bot merged 1 commit into
mainfrom
fix/tw775-trade-reminder-index
Oct 19, 2026
Merged

practical-bot merged 1 commit into
mainfrom
fix/tw775-trade-reminder-index

Conversation

@jcotillo

Copy link
Copy Markdown
Contributor

Why

cortex's Cron: Trade reminder sweep (trade-reminder-sweep, */15) has FAILED on every run on inngest.practical.works since at least 2026-10-08 (78/78 that day, 27/27 on 2026-10-18). Run 01M58NKH8MWZ2F5MW24WXBJ9H7 ends in 9 FAILED_PRECONDITION: The query requires an index. So no T-24h / T-2h appointment_reminder texts have gone to trade customers since.

The query (turnwrk-cortex src/services/tradeReminderSweep.ts:126, origin/main fc3026f):

cmms_workOrders.where('status','==','Scheduled')
  .where('scheduledDate','>=',start).where('scheduledDate','<=',end).limit(80)

No orderBy. The error's create_composite link decodes to collectionGroups/cmms_workOrders, fields status ASC, scheduledDate ASC, __name__ ASC, COLLECTION scope. Neither the file nor the live project has it: I read the live index list for project turnwrk and it matches this file, with no (status, scheduledDate).

Which file deploys. This file. firebase.json here points at firebase/firestore.indexes.json, and .github/workflows/deploy-firebase.yml (owner-triggered workflow_dispatch) is the only prod deploy. The copies at <app>/packages/shared/firebase/firestore.indexes.json (cortex, dispatch, which TurnWrk/cmms now redirects to, clean, restock) are vendored for emulators. sync-consumers.yml refreshes them from here, and dispatch's firebase.json says not to deploy from the app repos. Cortex has no firebase.json at all. So the fix goes here and only here. The vendored copies reach parity through the usual bot/sync-shared PRs, and consumer CI's integrity check enforces it.

What changed (net source lines: +24 JSON, 0 TS; the test file is +55 and excluded)

  • firebase/firestore.indexes.json: three COLLECTION-scope composites.
    • cmms_workOrders (status ASC, scheduledDate ASC): the sweep.
    • cmms_workOrders (assignedTechId ASC, createdAt DESC) and resupplyRequests (cleanerId ASC, createdAt DESC). Cortex runs both as where(==).orderBy('createdAt','desc').limit(1) in smsPhoneResolver.ts and telegramUserResolver.ts. Both are wrapped in try/catch that logs a warning, and the code comment even says "Composite index might be missing". Neither index is in the file or live, so inbound SMS / Telegram sender resolution has been quietly running without its "most recent work order / property" hint.
  • tests/firestoreIndexes.test.ts: a readable guard that checks each of the three is declared, and records which consumer query needs it.

Net positive because these are new declarations. Nothing existing does this job, so nothing could be removed.

What deliberately did NOT change

  • Sweep cadence (*/15): that is TURNWRK-776.
  • Sweep logic: no backlog guard added, because none is needed. pickReminder fires only when apptStart - now is within ±1h of 24h, or within ±1h of 2h. apptStart is 09:00 org-local on scheduledDate. A past appointment has a negative delta and never matches. An appointment whose 24h window passed during the outage (say 20h out) misses its 24h text and gets only its 2h text, at the right time. Each send writes customerReminder24hAt / customerReminder2hAt on the work order before sending, and checks it before the next send, so dedup is already on the doc. The query range (today-1 .. today+3) is relative to now. Result: once the index is live, the first run sends at most what would have been due in that 15-minute tick. It sends no burst of stale reminders.
  • Failure alerting (ticket AC 3: "a failing run of this cron alerts someone"): not in this PR. It is a cortex change (an onFailure / alert sink on cronFunctions), it adds shadow (failure) functions to a CONNECT app's registration, and it needs a destination choice. Filed separately (see the card comment).
  • Equality-only compound queries in cortex (missedCallService phone+orgId+status, telegramUserResolver orgId+telegramChatId, weather/load.ts orgId+date / orgId+scheduledDate, firestoreAdmin orgId+status): Firestore serves these by merging single-field indexes (no field overrides are declared, so single-field indexes are on), so they need no composite. I left them alone.
  • Other apps' queries (dispatch / clean / restock) were not audited. This sweep covered cortex only.

Proof

Guard seen to fail. Exact mutation: git stash -- firebase/firestore.indexes.json, then npx vitest run tests/firestoreIndexes.test.ts gives 3 failed / 3. git stash pop restores it, and the run gives 3 passed.

The Firestore emulator serves compound queries without composite indexes, so no emulator test can go red for a missing index. That is why this guard reads the declaration directly.

Local run: npm run typecheck is clean, and npx vitest run gives 78 files, 820 tests passed. npm run check:vendored fails locally only because of BSD sed on macOS (\1 not defined in the RE), in a script this PR does not touch. CI runs it on ubuntu.

Verification

Index deploys are not CI. After merge, an owner runs Actions -> Deploy Firebase rules -> Run workflow on main with targets=indexes and force=false. Additive only, so nothing is dropped. Equivalent CLI from this repo: npx firebase-tools@14 deploy --only firestore:indexes --project turnwrk. Tracked as a type:human-task card blocking TURNWRK-775.

Then:

  1. The three new indexes show READY on cmms_workOrders / resupplyRequests in project turnwrk.
  2. The next trade-reminder-sweep run on inngest.practical.works is COMPLETED with {scanned, sent24h, sent2h, skipped, errors}, and errors: 0.

Refs: TURNWRK-775

🤖 Generated with Claude Code

…URNWRK-775]

Symptom: cortex's "Cron: Trade reminder sweep" (*/15) failed every run on
inngest.practical.works since at least 2026-10-08 (78/78 that day, 27/27 on
2026-10-18), with `9 FAILED_PRECONDITION: The query requires an index`. No
T-24h / T-2h appointment_reminder text has gone to a trade customer since.

Mechanism: tradeReminderSweep.ts queries cmms_workOrders with
status == 'Scheduled' plus a scheduledDate range. That needs a composite
(status ASC, scheduledDate ASC); the error's console link decodes to exactly
that, COLLECTION scope. firebase/firestore.indexes.json here is the only file
that deploys to project turnwrk (deploy-firebase.yml); the copies under each
app's packages/shared/firebase are vendored for emulators. The live index
list matches this file, and it never had the pair.

The same sweep of cortex found two more compound queries with no index:
cmms_workOrders (assignedTechId ==, orderBy createdAt desc) and
resupplyRequests (cleanerId ==, orderBy createdAt desc), in
smsPhoneResolver.ts and telegramUserResolver.ts. Both catch the error and
log a warning, so inbound SMS / Telegram sender context has silently lost
its "most recent work order / property" hint. Declared both.

Rejected: creating the index from the console link. It fixes prod and
leaves the file stale, so the next `--force` index deploy would drop it.

Proof: the Firestore emulator serves compound queries without indexes, so
no emulator test can go red here. tests/firestoreIndexes.test.ts asserts the
three declarations directly; with the JSON change stashed it fails 3/3.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@jcotillo jcotillo added the merge:queued Hand this PR to the factory merge queue label Oct 19, 2026

@practical-bot practical-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

The diff is narrowly scoped to the canonical deploy source and a declaration guard. The three entries use COLLECTION scope and the exact field order/directions required by the cited queries. The test checks collection group, scope, field count, field paths, and ordering rather than merely checking that a name appears. No UI files changed, so the committed-screenshot requirement does not apply.

PR-body claim ledger

  • “cmms_workOrders (status ASC, scheduledDate ASC): the sweep.” PASS. The entry is present in firebase/firestore.indexes.json; tests/firestoreIndexes.test.ts passes its corresponding assertion.
  • “cmms_workOrders (assignedTechId ASC, createdAt DESC) and resupplyRequests (cleanerId ASC, createdAt DESC).” PASS. Both entries are present and their corresponding assertions pass.
  • “This file…is the only prod deploy.” PASS by repository inspection. firebase.json points at firebase/firestore.indexes.json, and .github/workflows/deploy-firebase.yml is the owner-triggered index deployment path.
  • “The vendored copies reach parity through the usual bot/sync-shared PRs, and consumer CI's integrity check enforces it.” PASS by repository inspection / CI. The workflow and the passing check:vendored step support this claim.
  • “No backlog guard added, because none is needed.” UNVERIFIED. The cited cortex implementation is outside this repository and no live cortex checkout was available in this review; this is a scope rationale, not a changed behavior in this PR.
  • “Failure alerting…is not in this PR…Filed separately.” PASS as scope accounting. The PR changes no alerting code, and the work-item record identifies the separate TURNWRK-779 split.
  • “Equality-only compound queries…need no composite.” UNVERIFIED. This requires reviewing the external cortex query set and Firestore production configuration; it is not established by this repository diff. No missing declaration was found within the stated PR scope.
  • “Other apps' queries…were not audited.” PASS. The body states the audit boundary explicitly, and this PR does not claim to modify those applications.
  • “Exact mutation: git stash -- firebase/firestore.indexes.json, then npx vitest run tests/firestoreIndexes.test.ts gives 3 failed / 3.” PASS, using an isolated equivalent mutation. I removed the three required declarations in a temporary copy (leaving the worktree untouched) and ran npx vitest run tests/firestoreIndexes.test.ts; output was 3 tests | 3 failed, MUTATION_EXIT=1, with all three assertions reporting expected undefined to be defined. The unmutated command passed: 1 passed, 3 passed.
  • “npm run typecheck is clean.” PASS. Ran npm run typecheck; exit 0.
  • “npx vitest run gives 78 files, 820 tests passed.” VERIFIED-BY-CI. Run 39181584877, job typecheck + unit, ran npm run test:run successfully on this head. I also ran the changed-surface test directly: npm run test:run -- tests/firestoreIndexes.test.ts; 1 passed, 3 passed.
  • “npm run check:vendored fails locally only because of BSD sed…CI runs it on ubuntu.” VERIFIED-BY-CI for the relevant claim. Run 39181584877, typecheck + unit, completed npm run check:vendored successfully.
  • “Index deploys are not CI…owner runs…with targets=indexes and force=false.” PASS by workflow inspection. .github/workflows/deploy-firebase.yml defines that manual gate and maps indexes to firestore:indexes without --force.
  • “The three new indexes show READY…The next trade-reminder-sweep run…is COMPLETED…” UNVERIFIED-BY-DESIGN. These are post-deploy production checks and cannot be answered from this pre-merge repository. The work item already carries needs:verification.
  • The CI rows were all checked: typecheck + unit, firestore rules behavior tests, and secret scan all passed on this head. The typecheck + unit job's steps included typecheck, unit tests, build, email typecheck, and vendored parity.

The acceptance criterion that is in this PR is met: the canonical source declares the required collection-scope composites. The production deployment and one-day green sweep remain post-merge verification, not review blockers. The alerting criterion is explicitly split to TURNWRK-779 rather than silently omitted.

REVIEW VERDICT: approve
NEEDS VERIFICATION: After TURNWRK-778 deploys firestore:indexes to project turnwrk, confirm the three indexes reach READY; then confirm the next trade-reminder-sweep run is COMPLETED and its output contains {scanned, sent24h, sent2h, skipped, errors} with errors: 0.

@practical-bot
practical-bot merged commit 90743b0 into main Oct 19, 2026
3 checks passed
@practical-bot
practical-bot deleted the fix/tw775-trade-reminder-index branch October 19, 2026 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge:queued Hand this PR to the factory merge queue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants