feat(phase-0): monorepo foundation, scaffolding, Firebase config, Terraform, CI#26
Merged
feat(phase-0): monorepo foundation, scaffolding, Firebase config, Terraform, CI#26
Conversation
Consolidates 7 approved design sections covering monorepo layout, shared packages, apps scaffolding, Firebase config, Terraform skeleton, CI pipeline, and tooling config. Includes formal deviations from plan §0 (barangay data → Phase 2 prerequisite; Capacitor native scaffolding split between Phase 6 smoke build and Phase 11 distribution). Driver for writing-plans skill invocation to produce the atomic task plan for Phase 0 implementation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…into internal _stubs.ts Stubs (all = never) are phase-future placeholders and must not pollute the public API. Consolidates all 8 into _stubs.ts and removes them from index.ts so they cannot be accidentally imported via the package barrel. Only branded, enums, and geo remain in the public surface. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ollisions
canonicalize() fell through to the plain-object branch for Map, Set, and
RegExp — all three return [] from Object.keys() and are rebuilt as {},
producing an identical hash to canonicalPayloadHash({}). For an
idempotency key function this is a silent correctness failure.
Fix: throw TypeError immediately when these types are encountered, before
the key-sorting path. Also adds @throws JSDoc to canonicalPayloadHash and
a test that verifies all three exotic types throw.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…II, no native dirs)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 4 <noreply@anthropic.com>
…ress - Rename vitest.config.ts → vitest.workspace.ts to prevent auto-discovery by packages without vitest configs (shared-types, shared-ui, apps, etc.) - Narrow vitest workspace to only shared-validators (only package with tests) - Remove test scripts from packages/apps that have no test files - Change root pnpm test to use vitest directly (auto-discovers workspace) - Add vitest.workspace.ts to ESLint ignores - Prettier-format all 14 docs/rules files (introduced by format:check) - Update docs/progress.md with Phase 0 completion - Append Phase 0 learnings to docs/learnings.md Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Exc1D, your pull request is larger than the review limit of 150000 diff characters
Contributor
📝 WalkthroughWalkthroughThis pull request establishes Phase 0 project foundation for Bantayog Alert, a disaster reporting and coordination platform. It introduces a complete monorepo structure with three React/Vite applications, shared packages for types/validators/UI/Firebase, Cloud Functions scaffolding, Terraform infrastructure-as-code, Firebase configuration, GitHub Actions CI/CD workflows, and developer tooling configuration alongside comprehensive role-based specifications and documentation. Changes
Sequence Diagram(s)The changes in this PR do not meet the criteria for sequence diagram generation. The additions are primarily scaffolding, configuration, and Phase 0 foundation work (mostly boilerplate files, package metadata, and static documentation). The functional logic is minimal (placeholder components, empty entry points, configuration declarations), and there is no significant multi-component interaction or sequential workflow to visualize. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Rationale: This is a large foundational PR with high heterogeneity—spanning configuration files (Terraform, GitHub Actions, ESLint, Prettier), infrastructure-as-code modules with distinct resource types, multiple independent applications (admin, citizen, responder) with similar but separate structures, shared packages with distinct responsibilities (types, validators, UI, Firebase), documentation, and role specifications. While individual files are often straightforward (package.json boilerplate, tsconfig extends), the breadth and variety of changes across multiple domains (frontend apps, backend functions, infrastructure, tooling, documentation) require diverse reasoning and validation for each cohort. Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 29
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/roles/responder-role-spec-v2.md (1)
274-300:⚠️ Potential issue | 🟡 MinorAdd a language identifier to the fenced code block (markdownlint MD040).
Line 274 opens a fenced block without a language, which can fail markdown lint checks.
💡 Suggested fix
-``` +```text 🟢 Status: AVAILABLE for dispatch [Set Unavailable] [Go Off-Duty]</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/roles/responder-role-spec-v2.mdaround lines 274 - 300, The fenced code
block that begins with "🟢 Status: AVAILABLE for dispatch" is missing a language
identifier which triggers markdownlint rule MD040; update that opening fence to
include a language (e.g., add "text" so it becomes ```text) for the block
containing the status buttons and the settings snippet to satisfy the linter and
preserve plain-text formatting.</details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed. Inline comments: In @.claude/rules/git-workflow.md: - Around line 49-53: The fenced code block containing the commit-message examples is missing a language hint (triggers markdownlint MD040); update that block (the triple-backtick block showing "feat(auth)...", "fix(reports)...", "docs(readme)...") to include a language specifier such as text (e.g., change ``` to ```text) so the commit-message examples are properly annotated for markdownlint. In @.firebaserc: - Around line 8-15: The targets section currently only defines hosting mappings for "bantayog-alert-dev" which causes deploys to "bantayog-alert-staging" and "bantayog-alert" to fail; add equivalent "hosting" mappings for "bantayog-alert-staging" and "bantayog-alert" with "citizen" and "admin" target arrays (using the actual Firebase Hosting site IDs for those environments), replace any placeholder site IDs with the real site IDs, then validate the .firebaserc locally (firebase target:list / firebase deploy --only hosting:TARGET) and obtain explicit approval before deploying to staging/production. In @.github/CODEOWNERS: - Around line 2-24: The CODEOWNERS file currently assigns single owner `@davidaviado` for many critical paths (e.g., entries like "/infra/terraform/", "/functions/", "/infra/firebase/firestore.rules", and the shared packages under "/packages/shared-*"); update these entries to include a secondary owner or team (for example add a backup username or a GitHub team such as `@org/security-team`) so each critical path contains two owners separated by a space, ensuring no single-person bottleneck for review/approvals. In @.github/workflows/ci.yml: - Around line 137-153: The terraform-validate job is iterating over matrix env (dev/staging/prod) but runs terraform validate without loading environment-specific variable files, causing missing required variables; update the terraform-validate steps (job name: terraform-validate) to pass the correct tfvars for each matrix value by using the matrix.env to supply the var file (e.g., add a run that sets VARFILE="terraform.${{ matrix.env }}.tfvars" and invoke terraform validate with -var-file="$VARFILE" or otherwise load the corresponding env-specific tfvars before running terraform validate) so terraform validate uses the env-specific values for project_id/project_number/env/state_bucket. In @.github/workflows/codeql.yml: - Around line 24-29: Update the CodeQL GitHub Action versions from v3 to v4 by replacing the uses entries for github/codeql-action/init@v3 and github/codeql-action/analyze@v3 with github/codeql-action/init@v4 and github/codeql-action/analyze@v4 respectively; ensure the rest of the step inputs (languages, queries) remain unchanged and run the workflow to verify compatibility with the v4 action. In `@apps/admin-desktop/vite.config.ts`: - Line 7: The Vite build config currently enables sourcemaps by default (build: { outDir: 'dist', sourcemap: true }), which exposes source internals; change the build.sourcemap setting to false for production builds (or make it conditional on an environment variable like NODE_ENV !== 'production' or a VITE_ENABLE_SOURCEMAP flag) so .map files are not emitted for deployed artifacts, keeping outDir and other build settings intact. In `@apps/citizen-pwa/public/manifest.webmanifest`: - Around line 10-23: The manifest declares PWA icons under the "icons" array (src: "/icons/icon-192.png" and "/icons/icon-512.png") but those files and the icons/ directory are missing; create a public/icons/ directory and add appropriately sized PNG images named icon-192.png (192x192) and icon-512.png (512x512), ensure they are valid PNGs and accessible at those paths so the entries in manifest.webmanifest resolve correctly. In `@apps/citizen-pwa/vite.config.ts`: - Line 7: The config currently sets build.sourcemap unconditionally; change the exported defineConfig to be a function that accepts the Vite mode (e.g., defineConfig(({ mode }) => ({ ... }))) and gate the sourcemap flag based on mode (e.g., set build.sourcemap to false when mode === 'production' and true otherwise). Update the config where build: { outDir: 'dist', sourcemap: true } appears so the sourcemap value is computed from the received mode rather than hardcoded. In `@docs/learnings.md`: - Around line 224-230: Update the heading text "Two-part" to accurately reflect the three listed changes (rename vitest.config.ts → vitest.workspace.ts, removal of test scripts except packages/shared-validators, and changing root pnpm test to vitest run) — e.g., change "Two-part" to "Three-part" or reword to "Summary" so the count matches the three numbered items. In `@docs/superpowers/specs/2026-04-17-phase-0-design.md`: - Line 39: Several fenced code blocks in the document are unlabeled (they appear as ``` with no language) which triggers MD040; update each unlabeled fence to include an appropriate language identifier (e.g., ```text, ```rules, ```hcl, ```yaml) so syntax highlighting and linting pass. Locate the unlabeled fences by searching for standalone ``` occurrences in the spec and replace each with the corresponding labeled fence based on the block content (choose text for plain examples, rules for rule sets, hcl for Terraform-like snippets, yaml for configuration blocks). Ensure opening fences are updated and closing fences remain ``` unchanged. In `@eslint.config.js`: - Around line 39-40: Replace the untracked TODO comment before the '@typescript-eslint/no-unsafe-assignment' rule with either a proper ticket reference or remove it: update the comment to something like "TODO(AB-1234): enable `@typescript-eslint/no-unsafe-assignment`" (use the actual ticket ID) or delete the TODO line entirely and, if you need to defer enabling the rule, add a short explanatory comment referencing the ticket; ensure the change is applied immediately adjacent to the '@typescript-eslint/no-unsafe-assignment' configuration entry so the intent is traceable. - Around line 46-49: The selector currently only matches CallExpression[callee.property.name='doc'] TemplateLiteral and therefore misses direct calls like doc`...`; update the rule selector to also match Identifier callees by adding an alternative for CallExpression[callee.name='doc'] TemplateLiteral (or combine with an OR like "CallExpression[callee.property.name='doc'], CallExpression[callee.name='doc'] TemplateLiteral") so both member-expression calls and direct doc(...) / doc`...` usages are caught; adjust the rule definition surrounding the existing selector string accordingly. In `@firebase.json`: - Around line 16-25: Update the PR/deploy checklist and PR template to include explicit rollout/rollback notes whenever the "firestore", "database", or "storage" sections (and their linked rule files: infra/firebase/firestore.rules, infra/firebase/database.rules.json, infra/firebase/storage.rules) are modified: add an "emulator-first validation" step (validate changes locally in the Firebase emulator), a required "staging approval" gate before production deploy, and include a clear rollback deploy command and instructions (and where to paste it) in the PR description so reviewers can copy it into the thread if needed; ensure these checklist items appear automatically for PRs that touch those rule/index files. In `@functions/package.json`: - Line 14: The deploy script in package.json ("deploy") doesn't run a build step and can deploy stale code; update the "deploy" npm script so it runs the same build step as "serve" and "shell" (i.e., run "pnpm build" first) before invoking "firebase deploy --only functions" so deployments always use freshly built artifacts. In `@infra/firebase/firestore.rules`: - Around line 18-20: The role() helper should defensively handle unauthenticated requests instead of directly accessing request.auth.token.role; update the role() function to check request.auth and request.auth.token (e.g., return null or an empty string if missing) so callers can safely use it without always invoking isAuthed(); modify any dependent uses if they expect a non-null string (or keep current callers unchanged if they already call isAuthed()), and ensure the change is implemented inside the existing role() function to centralize the defensive check. In `@infra/terraform/envs/dev/terraform.tfvars`: - Line 2: The project_number Terraform variable lacks validation and can contain the bootstrap placeholder "REPLACE_WITH_PROJECT_NUMBER_AT_BOOTSTRAP"; add a validation block to the variable definition for project_number (the variable named project_number in infra/terraform/variables.tf) that enforces a numeric-only pattern (e.g., regex for digits) and a helpful error message rejecting non-numeric placeholders so incorrect tfvars are rejected early during plan/apply. In `@infra/terraform/envs/prod/terraform.tfvars`: - Line 2: The prod tfvars contains an unresolved placeholder for project_number ("REPLACE_WITH_PROJECT_NUMBER_AT_BOOTSTRAP"); add a guard to prevent applying with that value by adding a validation block to the variable "project_number" (in your Terraform variables definition) that rejects the exact placeholder string and returns a clear error message, or implement a CI pre-check that fails if the prod terraform.tfvars contains "REPLACE_WITH_PROJECT_NUMBER_AT_BOOTSTRAP"; reference the variable name project_number and the placeholder string when implementing the check or validation. In `@infra/terraform/envs/staging/terraform.tfvars`: - Line 2: Add a validation block to the Terraform variable "project_number" in variables.tf to reject the bootstrap sentinel value and invalid formats: update the variable "project_number" declaration to include a validation with a rule that the value is not equal to "REPLACE_WITH_PROJECT_NUMBER_AT_BOOTSTRAP" and matches the expected numeric/project ID pattern (e.g., a 12-digit numeric project number or other org-specific regex), and add a clear error_message explaining the rejection; mirror the style used by the existing "env" validation block so Terraform plan/apply will fail if the placeholder or malformed IDs are present. In `@infra/terraform/modules/iam/main.tf`: - Line 6: The module description currently claims "Grants read/write to Firestore, Storage, FCM, Secret Manager." but the IAM module does not bind any Storage roles; update the module's description string in main.tf (the description = "...") to accurately reflect only the services it actually grants (e.g., remove "Storage read/write" or replace with the correct bound role), or alternatively add the missing Storage IAM binding/resource if Storage read/write was intended; ensure the description and actual bindings in this IAM module are consistent. - Around line 51-55: The CI service account is currently granted roles/iam.serviceAccountUser at the project level (resource google_project_iam_member.ci_sa_user), which is too broad; change the resource to a service-account-scoped binding by replacing google_project_iam_member.ci_sa_user with google_service_account_iam_member and scope it to the runtime service account (google_service_account.functions) instead of the project. Update the resource to use service_account_id = google_service_account.functions.name (or .email as required by the provider), keep member = "serviceAccount:${google_service_account.ci_deploy.email}" and role = "roles/iam.serviceAccountUser", and remove the project attribute so the impersonation right is limited to the Functions runtime SA. In `@infra/terraform/modules/iam/variables.tf`: - Around line 6-9: The module-level variable "env" currently lacks validation; update the variable "env" in this module by adding a validation block that enforces allowed values ["dev","staging","prod"] and provides a clear error message, so the variable declaration for "env" (variable "env") becomes self-validating and matches the root-level constraint; ensure the validation uses Terraform's validation/condition and error_message fields. In `@infra/terraform/modules/pubsub/main.tf`: - Around line 10-21: Add support for customer-managed encryption by declaring a new optional string variable pubsub_kms_key_name (e.g., in variables.tf) and wiring it into the google_pubsub_topic.dead_letters resource: add the kms_key_name = var.pubsub_kms_key_name attribute on the resource (or conditionally set it only when the variable is non-empty), ensuring the module exposes pubsub_kms_key_name so dead-letter topics use the provided CMEK key. In `@infra/terraform/README.md`: - Around line 68-70: The README shows a literal secret inserted via echo -n "ACTUAL_VALUE" piped into gcloud secrets versions add SEMAPHORE_API_KEY --data-file=- which can leak via shell history; update the documented flow to read the secret from stdin or a silent prompt instead (e.g., instruct users to use a secure prompt like read -s or to paste into stdin) and keep the gcloud invocation using the --data-file=- and SEMAPHORE_API_KEY tokens so the example demonstrates non-echoed, stdin-based secret upload. In `@packages/shared-data/README.md`: - Around line 5-9: The README lists data sources (PSA, PhilGIS, Province of Camarines Norte PDRRMO) as "pending licensing" but lacks a tracked action; create a Phase 2 tracking issue to verify licensing/usage rights for each source and update the README to reference that issue and the verification checklist; specifically mention PSA, PhilGIS, and PDRRMO by name in the issue and add acceptance criteria (license type, allowed uses, source URL, and contact) so the barangay dataset integration is gated until the tracking issue is resolved. In `@packages/shared-types/package.json`: - Around line 6-15: The package.json exposes runtime entrypoints ("main"/"types"/"exports") pointing at ./src/index.ts while the build script ("build": "tsc --emitDeclarationOnly --outDir lib") only emits type declarations, and index.ts imports ./branded.js so runtime fails; fix by either making the package types-only or producing JS: update the build to emit JS (remove --emitDeclarationOnly or change to tsc --outDir lib) and change package.json "main" and "exports" to point to ./lib/index.js (and keep "types" pointing to ./lib/index.d.ts), or alternatively remove runtime exports from src/index.ts (the cast helpers like asReportId, asDispatchId, etc.) so the package is type-only and then set package.json to only expose types (remove "main"/"exports" or point them to type-only entry) — ensure imports in src/index.ts reference .ts/.js consistently so Node ESM resolution matches emitted artifacts. In `@packages/shared-types/src/geo.ts`: - Line 14: The asGeohash function currently blindly casts any string to Geohash; update asGeohash to validate the input string against a geohash format (e.g., allowed base32 chars and length constraints) before branding: if the input matches the geohash regex allow the cast (return v as Geohash), otherwise throw a clear error or return a safe failure (e.g., undefined or Result) so invalid values cannot be silently accepted; update callers if you change the return shape. Ensure references to asGeohash and the Geohash type are the touchpoints for this change. In `@packages/shared-validators/src/idempotency.ts`: - Around line 24-43: Update the canonicalize function to handle Date objects explicitly: inside canonicalize, detect value instanceof Date and return value.toISOString() (so dates canonicalize deterministically) instead of falling through to object handling; keep the existing Map/Set/RegExp rejection behavior and leave other exotic types unchanged for now. This change should be applied in the canonicalize function to ensure Date values are intentionally and deterministically serialized when computing payload hashes. In `@tsconfig.eslint.json`: - Line 11: The tsconfig used by ESLint includes an incorrect/nonexistent root entry "vitest.config.ts" so the actual root Vitest workspace file (vitest.workspace.ts) isn't compiled; update the "include" array in tsconfig.eslint.json to remove "vitest.config.ts" and add "vitest.workspace.ts" (keeping "eslint.config.js") so ESLint types cover the root Vitest workspace file. In `@turbo.json`: - Around line 26-30: The test:rules Turbo task currently lacks an inputs array, so add an "inputs" entry to the "test:rules" task in turbo.json (the task named "test:rules") that lists the rule files and test sources to allow proper cache invalidation; for example include filenames like "firestore.rules", "database.rules.json", any rules folder (e.g., "rules/**"), and the test harness/spec files (e.g., "test/**" or "tests/**") so Turbo can cache/invalidates runs when those files change. --- Outside diff comments: In `@docs/roles/responder-role-spec-v2.md`: - Around line 274-300: The fenced code block that begins with "🟢 Status: AVAILABLE for dispatch" is missing a language identifier which triggers markdownlint rule MD040; update that opening fence to include a language (e.g., add "text" so it becomes ```text) for the block containing the status buttons and the settings snippet to satisfy the linter and preserve plain-text formatting.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID:
3c17b4dd-d5b4-4522-8fd4-7566f4a7aaf6⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml📒 Files selected for processing (127)
.claude/rules/coding-style.md.claude/rules/git-workflow.md.claude/rules/security.md.claude/rules/testing.md.editorconfig.firebaserc.gitattributes.github/CODEOWNERS.github/dependabot.yml.github/workflows/ci.yml.github/workflows/codeql.yml.gitignore.husky/pre-commit.lintstagedrc.json.nvmrc.prettierignore.prettierrc.vscode/extensions.jsonREADME.mdapps/admin-desktop/index.htmlapps/admin-desktop/package.jsonapps/admin-desktop/public/manifest.webmanifestapps/admin-desktop/src/App.module.cssapps/admin-desktop/src/App.tsxapps/admin-desktop/src/main.tsxapps/admin-desktop/src/vite-env.d.tsapps/admin-desktop/tsconfig.jsonapps/admin-desktop/vite.config.tsapps/citizen-pwa/index.htmlapps/citizen-pwa/package.jsonapps/citizen-pwa/public/manifest.webmanifestapps/citizen-pwa/src/App.module.cssapps/citizen-pwa/src/App.tsxapps/citizen-pwa/src/main.tsxapps/citizen-pwa/src/vite-env.d.tsapps/citizen-pwa/tsconfig.jsonapps/citizen-pwa/vite.config.tsapps/responder-app/capacitor.config.tsapps/responder-app/index.htmlapps/responder-app/package.jsonapps/responder-app/src/App.module.cssapps/responder-app/src/App.tsxapps/responder-app/src/main.tsxapps/responder-app/src/vite-env.d.tsapps/responder-app/tsconfig.jsonapps/responder-app/vite.config.tsdocs/learnings.mddocs/progress.mddocs/roles/agency-admin-role-spec-v2.mddocs/roles/citizen-role-spec-v2.mddocs/roles/municipal-admin-role-spec-v2.mddocs/roles/provincial-superadmin-role-spec-v2.mddocs/roles/responder-role-spec-v2.mddocs/superpowers/specs/2026-04-17-phase-0-design.mdeslint.config.jsfirebase.jsonfunctions/package.jsonfunctions/src/index.tsfunctions/tsconfig.jsoninfra/firebase/database.rules.jsoninfra/firebase/firestore.indexes.jsoninfra/firebase/firestore.rulesinfra/firebase/storage.rulesinfra/terraform/.terraform.lock.hclinfra/terraform/README.mdinfra/terraform/backend.tfinfra/terraform/envs/dev/backend.hclinfra/terraform/envs/dev/terraform.tfvarsinfra/terraform/envs/prod/backend.hclinfra/terraform/envs/prod/terraform.tfvarsinfra/terraform/envs/staging/backend.hclinfra/terraform/envs/staging/terraform.tfvarsinfra/terraform/main.tfinfra/terraform/modules/firebase-project/main.tfinfra/terraform/modules/firebase-project/outputs.tfinfra/terraform/modules/firebase-project/variables.tfinfra/terraform/modules/iam/main.tfinfra/terraform/modules/iam/outputs.tfinfra/terraform/modules/iam/variables.tfinfra/terraform/modules/pubsub/main.tfinfra/terraform/modules/pubsub/outputs.tfinfra/terraform/modules/pubsub/variables.tfinfra/terraform/modules/secret-manager/main.tfinfra/terraform/modules/secret-manager/outputs.tfinfra/terraform/modules/secret-manager/variables.tfinfra/terraform/outputs.tfinfra/terraform/providers.tfinfra/terraform/variables.tfinfra/terraform/versions.tfpackage.jsonpackages/shared-data/README.mdpackages/shared-data/data/.gitkeeppackages/shared-data/package.jsonpackages/shared-data/src/index.tspackages/shared-data/tsconfig.jsonpackages/shared-firebase/package.jsonpackages/shared-firebase/src/index.tspackages/shared-firebase/tsconfig.jsonpackages/shared-sms-parser/package.jsonpackages/shared-sms-parser/src/index.tspackages/shared-sms-parser/tsconfig.jsonpackages/shared-types/package.jsonpackages/shared-types/src/_stubs.tspackages/shared-types/src/branded.tspackages/shared-types/src/enums.tspackages/shared-types/src/geo.tspackages/shared-types/src/index.tspackages/shared-types/tsconfig.jsonpackages/shared-ui/package.jsonpackages/shared-ui/src/index.tspackages/shared-ui/src/theme.csspackages/shared-ui/tsconfig.jsonpackages/shared-validators/package.jsonpackages/shared-validators/src/idempotency.test.tspackages/shared-validators/src/idempotency.tspackages/shared-validators/src/index.tspackages/shared-validators/tsconfig.jsonpackages/shared-validators/vitest.config.tspnpm-workspace.yamlprd/bantayog-alert-architecture-spec-v8.mdprd/bantayog-alert-implementation-plan-v1.0.mdprd/bantayog-alert-prd-v1.0.mdtsconfig.base.jsontsconfig.eslint.jsontsconfig.jsonturbo.jsonvitest.workspace.ts
Comment on lines
49
to
53
| ``` | ||
| feat(auth): add OAuth2 login | ||
| fix(reports): correct date filtering | ||
| docs(readme): update installation steps | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the fenced code block to satisfy markdownlint (MD040).
Use a language hint (e.g., text) for the commit-message examples block.
Doc lint fix
-```
+```text
feat(auth): add OAuth2 login
fix(reports): correct date filtering
docs(readme): update installation steps</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 49-49: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/rules/git-workflow.md around lines 49 - 53, The fenced code block
containing the commit-message examples is missing a language hint (triggers
markdownlint MD040); update that block (the triple-backtick block showing
"feat(auth)...", "fix(reports)...", "docs(readme)...") to include a language
specifier such as text (e.g., change ``` to ```text) so the commit-message
examples are properly annotated for markdownlint.
Comment on lines
+8
to
+15
| "targets": { | ||
| "bantayog-alert-dev": { | ||
| "hosting": { | ||
| "citizen": ["bantayog-citizen-dev"], | ||
| "admin": ["bantayog-admin-dev"] | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify every non-default project alias has hosting target mappings for citizen/admin.
project_ids=$(jq -r '.projects | to_entries[] | select(.key!="default") | .value' .firebaserc | sort -u)
missing=0
for pid in $project_ids; do
if ! jq -e --arg pid "$pid" '.targets[$pid].hosting.citizen and .targets[$pid].hosting.admin' .firebaserc >/dev/null; then
echo "Missing hosting target mapping for project: $pid"
missing=1
fi
done
exit $missingRepository: Exc1D/bantayog-alert
Length of output: 189
Add staging/prod hosting target mappings to prevent deploy-target failures.
The .firebaserc targets section only defines hosting mappings for bantayog-alert-dev. Both bantayog-alert-staging and bantayog-alert (prod) lack corresponding citizen and admin mappings, causing Firebase CLI hosting deploys to those projects to fail when using named targets.
🛠️ Suggested patch
"targets": {
"bantayog-alert-dev": {
"hosting": {
"citizen": ["bantayog-citizen-dev"],
"admin": ["bantayog-admin-dev"]
}
+ },
+ "bantayog-alert-staging": {
+ "hosting": {
+ "citizen": ["<staging-citizen-site-id>"],
+ "admin": ["<staging-admin-site-id>"]
+ }
+ },
+ "bantayog-alert": {
+ "hosting": {
+ "citizen": ["<prod-citizen-site-id>"],
+ "admin": ["<prod-admin-site-id>"]
+ }
}
}Substitute the placeholder site IDs with actual Firebase Hosting site IDs, test the configuration locally, and obtain explicit approval before deploying to staging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.firebaserc around lines 8 - 15, The targets section currently only defines
hosting mappings for "bantayog-alert-dev" which causes deploys to
"bantayog-alert-staging" and "bantayog-alert" to fail; add equivalent "hosting"
mappings for "bantayog-alert-staging" and "bantayog-alert" with "citizen" and
"admin" target arrays (using the actual Firebase Hosting site IDs for those
environments), replace any placeholder site IDs with the real site IDs, then
validate the .firebaserc locally (firebase target:list / firebase deploy --only
hosting:TARGET) and obtain explicit approval before deploying to
staging/production.
Comment on lines
+2
to
+24
| * @davidaviado | ||
|
|
||
| # Security-surface files — extra scrutiny required | ||
| /infra/firebase/firestore.rules @davidaviado | ||
| /infra/firebase/database.rules.json @davidaviado | ||
| /infra/firebase/storage.rules @davidaviado | ||
| /infra/firebase/firestore.indexes.json @davidaviado | ||
|
|
||
| # Infrastructure | ||
| /infra/terraform/ @davidaviado | ||
|
|
||
| # Cloud Functions — runs with elevated privileges | ||
| /functions/ @davidaviado | ||
|
|
||
| # Shared packages — changes ripple to every app | ||
| /packages/shared-validators/ @davidaviado | ||
| /packages/shared-firebase/ @davidaviado | ||
| /packages/shared-types/ @davidaviado | ||
|
|
||
| # CI + tooling | ||
| /.github/ @davidaviado | ||
| /eslint.config.js @davidaviado | ||
| /tsconfig.base.json @davidaviado No newline at end of file |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding a secondary owner/team for critical paths.
Line 2 and Lines 5-24 assign a single owner everywhere. This is workable short-term, but it creates a review bottleneck and continuity risk for protected branches. Add at least one backup owner or GitHub team for security/infra/function paths.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/CODEOWNERS around lines 2 - 24, The CODEOWNERS file currently
assigns single owner `@davidaviado` for many critical paths (e.g., entries like
"/infra/terraform/", "/functions/", "/infra/firebase/firestore.rules", and the
shared packages under "/packages/shared-*"); update these entries to include a
secondary owner or team (for example add a backup username or a GitHub team such
as `@org/security-team`) so each critical path contains two owners separated by a
space, ensuring no single-person bottleneck for review/approvals.
Comment on lines
+137
to
+153
| terraform-validate: | ||
| name: Terraform Validate | ||
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| env: [dev, staging, prod] | ||
| defaults: | ||
| run: | ||
| working-directory: infra/terraform | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: hashicorp/setup-terraform@v3 | ||
| with: | ||
| terraform_version: 1.9.8 | ||
| - run: terraform fmt -recursive -check | ||
| - run: terraform init -backend=false | ||
| - run: terraform validate |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if env-specific tfvars files exist and what variables they define
fd -t f 'terraform.tfvars' infra/terraform --exec cat {}Repository: Exc1D/bantayog-alert
Length of output: 664
🏁 Script executed:
#!/bin/bash
# Find all terraform.tfvars files with full paths
find infra/terraform -name 'terraform.tfvars' -type f
# Check the directory structure
tree -L 3 infra/terraform 2>/dev/null || find infra/terraform -type d | head -20Repository: Exc1D/bantayog-alert
Length of output: 910
🏁 Script executed:
#!/bin/bash
# Check the variables.tf to understand variable requirements
cat infra/terraform/variables.tfRepository: Exc1D/bantayog-alert
Length of output: 755
Terraform validate runs without environment-specific variable files.
The terraform-validate job matrices over dev, staging, prod but doesn't pass the corresponding terraform.tfvars file. Since variables.tf defines project_id, project_number, env, and state_bucket as required variables without defaults, terraform validate will fail without these values.
Update to load the env-specific tfvars:
Suggested fix
- run: terraform fmt -recursive -check
- - run: terraform init -backend=false
- - run: terraform validate
+ - run: terraform init -backend=false -var-file=envs/${{ matrix.env }}/terraform.tfvars
+ - run: terraform validate -var-file=envs/${{ matrix.env }}/terraform.tfvars🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 137 - 153, The terraform-validate job
is iterating over matrix env (dev/staging/prod) but runs terraform validate
without loading environment-specific variable files, causing missing required
variables; update the terraform-validate steps (job name: terraform-validate) to
pass the correct tfvars for each matrix value by using the matrix.env to supply
the var file (e.g., add a run that sets VARFILE="terraform.${{ matrix.env
}}.tfvars" and invoke terraform validate with -var-file="$VARFILE" or otherwise
load the corresponding env-specific tfvars before running terraform validate) so
terraform validate uses the env-specific values for
project_id/project_number/env/state_bucket.
Comment on lines
+24
to
+29
| - uses: actions/checkout@v4 | ||
| - uses: github/codeql-action/init@v3 | ||
| with: | ||
| languages: ${{ matrix.language }} | ||
| queries: security-and-quality | ||
| - uses: github/codeql-action/analyze@v3 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect workflow action pinning style across repo
fd -i '*.yml' .github/workflows -x sh -c 'echo "=== {} ==="; rg -n "uses:\\s+" "{}"'
# Focus on floating tags used in CodeQL workflow
rg -n "uses:\\s+[^@]+@v[0-9]+" .github/workflows/codeql.ymlRepository: Exc1D/bantayog-alert
Length of output: 497
🏁 Script executed:
# List all workflow files to see overall pinning practices
fd -type f -name '*.yml' .github/workflows/ | head -20Repository: Exc1D/bantayog-alert
Length of output: 234
🏁 Script executed:
# Check how other workflows in the repo handle action pinning
fd -type f -name '*.yml' .github/workflows/ -exec sh -c 'echo "=== {} ==="; head -50 "{}" | grep -n "uses:"' \;Repository: Exc1D/bantayog-alert
Length of output: 234
🌐 Web query:
GitHub CodeQL action versioning best practices supply chain security pinning
💡 Result:
For GitHub CodeQL actions (github/codeql-action/init, analyze, etc.), the official best practice for versioning balances supply chain security with functionality. Use major version tags like github/codeql-action/init@v4 in workflows. This provides automatic updates to the latest patch and minor releases within the major version, including bug fixes, new features, and updated CodeQL CLI bundles, while avoiding breaking changes. Major version tags are immutable and trusted for GitHub's own actions, mitigating supply chain risks from tag movement (unlike third-party actions, where full commit SHA pinning is recommended). Examples from official workflows and docs use @v2 or @v4. If pinning to a specific full commit SHA (e.g., github/codeql-action@de0fac2 # v4.35.1) or patch version (e.g., @v4.35.1) for stricter immutability, keep it updated regularly using Dependabot or manual reviews to avoid losing features from server-side flags or deprecated CodeQL versions. Old pinned versions may break as server-side features are removed (e.g., CodeQL Action v3 deprecation warning in 2025). Default setup (recommended for most repos) handles versioning automatically without manual workflow configuration. For advanced setups, supported versions are v4 (latest) and v3. Example workflow snippet: - name: Initialize CodeQL uses: github/codeql-action/init@v4 with: languages: ${{ matrix.language }} - name: Autobuild uses: github/codeql-action/autobuild@v4 - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@v4
Citations:
- 1: https://github.com/github/codeql-action
- 2: https://github.com/github/codeql-action/
- 3: https://github.com/github/codeql-action/blob/main/README.md
- 4: docs: guidance on keeping the CodeQL Action up to date github/codeql-action#3499
- 5: https://github.com/github/codeql-action/blob/main/CHANGELOG.md
- 6: https://codeql.github.com/codeql-query-help/actions/actions-unpinned-tag/
🏁 Script executed:
# List all workflow files
find .github/workflows -name '*.yml' -type fRepository: Exc1D/bantayog-alert
Length of output: 117
🏁 Script executed:
# Check how other workflows handle action versioning
find .github/workflows -name '*.yml' -type f -exec sh -c 'echo "=== {} ==="; grep -n "uses:" "{}"' \;Repository: Exc1D/bantayog-alert
Length of output: 1217
🌐 Web query:
GitHub CodeQL action v3 deprecation end of life v4 upgrade 2025
💡 Result:
GitHub CodeQL Action v3 is scheduled for deprecation in December 2026, coinciding with the end of support for GitHub Enterprise Server (GHES) 3.19. CodeQL Action v4 was released on October 7, 2025, and runs on Node.js 24 runtime. Key details: - Default code scanning setups on github.com and GHES 3.20+ automatically use v4; no action needed. - For custom workflows or older GHES versions, manually update from @v3 to @v4 in .github/workflows files: - github/codeql-action/init@v3 → @v4 - github/codeql-action/autobuild@v3 → @v4 - github/codeql-action/analyze@v3 → @v4 - github/codeql-action/upload-sarif@v3 → @v4 - GHES 3.18 and older cannot run v4 due to Node.js 24 incompatibility. - Post-deprecation (Dec 2026), v3 receives no updates; new CodeQL features only in v4. - Dependabot can automate upgrades. - Warnings already appear in v3 workflows as of late 2025. As of April 17, 2026, v3 remains supported until December 2026; upgrade recommended now for latest features and security.
Citations:
- 1: https://github.blog/changelog/2025-10-28-upcoming-deprecation-of-codeql-action-v3/%22
- 2: https://github.blog/changelog/2025-10-28-upcoming-deprecation-of-codeql-action-v3/
- 3:
CodeQL Action v3 will be deprecated in December 2026actions/checkout#2403 - 4: [GitHub Actions] CodeQL Action v3 will be deprecated in December 2026 DSpace/DSpace#11998
- 5: Update CodeQL Action to v4 (v3 deprecated Dec 2026) netbox-community/netbox#21156
- 6: https://github.com/github/codeql-action/blob/v4.31.4/CHANGELOG.md
Upgrade CodeQL actions from v3 to v4.
CodeQL Action v3 is scheduled for deprecation (December 2026). Upgrade github/codeql-action/init@v3 and github/codeql-action/analyze@v3 to @v4 to align with the repo's versioning pattern (ci.yml uses v4) and avoid future breakage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/codeql.yml around lines 24 - 29, Update the CodeQL GitHub
Action versions from v3 to v4 by replacing the uses entries for
github/codeql-action/init@v3 and github/codeql-action/analyze@v3 with
github/codeql-action/init@v4 and github/codeql-action/analyze@v4 respectively;
ensure the rest of the step inputs (languages, queries) remain unchanged and run
the workflow to verify compatibility with the v4 action.
Comment on lines
+6
to
+15
| "main": "./src/index.ts", | ||
| "types": "./src/index.ts", | ||
| "exports": { | ||
| ".": "./src/index.ts" | ||
| }, | ||
| "scripts": { | ||
| "lint": "eslint src", | ||
| "typecheck": "tsc --noEmit", | ||
| "build": "tsc --emitDeclarationOnly --outDir lib" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Runtime-export candidates in shared-types =="
rg -nP --type=ts -C1 '^\s*export\s+(enum|const|function|class)\b' packages/shared-types/src || true
echo
echo "== Non-type imports of `@bantayog/shared-types` =="
rg -nP --glob '*.{ts,tsx,js,jsx,mts,cts,mjs,cjs}' -C1 \
'^\s*import(?!\s+type\b).*from\s*["'"'"']@bantayog/shared-types["'"'"']' || true
echo
echo "== CommonJS runtime imports =="
rg -nP --glob '*.{ts,tsx,js,jsx,mts,cts,mjs,cjs}' -C1 \
'require\(\s*["'"'"']@bantayog/shared-types["'"'"']\s*\)' || trueRepository: Exc1D/bantayog-alert
Length of output: 1535
🏁 Script executed:
cat -n packages/shared-types/package.jsonRepository: Exc1D/bantayog-alert
Length of output: 511
🏁 Script executed:
cat -n packages/shared-types/src/index.tsRepository: Exc1D/bantayog-alert
Length of output: 232
🏁 Script executed:
head -n 25 packages/shared-types/src/branded.tsRepository: Exc1D/bantayog-alert
Length of output: 1561
Fix package entrypoint configuration: main/types point to TS source but build emits declarations only.
Lines 6-10 expose ./src/index.ts as runtime entrypoints, while Line 14 builds with --emitDeclarationOnly into lib/. Additionally, index.ts (Line 1-3) imports from .js files that never get compiled. This breaks in any non-TS-aware runtime—Node ESM will fail to resolve ./branded.js when the actual files are branded.ts.
Runtime-exported cast helpers (asReportId, asDispatchId, etc.) exist but are inaccessible at runtime. Either configure as type-only (remove runtime exports, keep branded types only) or fix the build to emit JavaScript and update main to ./lib/index.js.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared-types/package.json` around lines 6 - 15, The package.json
exposes runtime entrypoints ("main"/"types"/"exports") pointing at
./src/index.ts while the build script ("build": "tsc --emitDeclarationOnly
--outDir lib") only emits type declarations, and index.ts imports ./branded.js
so runtime fails; fix by either making the package types-only or producing JS:
update the build to emit JS (remove --emitDeclarationOnly or change to tsc
--outDir lib) and change package.json "main" and "exports" to point to
./lib/index.js (and keep "types" pointing to ./lib/index.d.ts), or alternatively
remove runtime exports from src/index.ts (the cast helpers like asReportId,
asDispatchId, etc.) so the package is type-only and then set package.json to
only expose types (remove "main"/"exports" or point them to type-only entry) —
ensure imports in src/index.ts reference .ts/.js consistently so Node ESM
resolution matches emitted artifacts.
| } | ||
|
|
||
| export type Geohash = string & { readonly __brand: 'Geohash' } | ||
| export const asGeohash = (v: string): Geohash => v as Geohash |
Contributor
There was a problem hiding this comment.
Validate geohash input before branding.
On Line 14, asGeohash blindly casts any string, so invalid geohash values can silently enter downstream logic.
Proposed fix
export type Geohash = string & { readonly __brand: 'Geohash' }
-export const asGeohash = (v: string): Geohash => v as Geohash
+const GEOHASH_RE = /^[0123456789bcdefghjkmnpqrstuvwxyz]{1,12}$/i
+export const asGeohash = (v: string): Geohash => {
+ if (!GEOHASH_RE.test(v)) throw new TypeError('Invalid geohash')
+ return v.toLowerCase() as Geohash
+}As per coding guidelines: “Defensive Programming: Assume external input is malicious/broken. Validate at the boundary.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const asGeohash = (v: string): Geohash => v as Geohash | |
| export type Geohash = string & { readonly __brand: 'Geohash' } | |
| const GEOHASH_RE = /^[0123456789bcdefghjkmnpqrstuvwxyz]{1,12}$/i | |
| export const asGeohash = (v: string): Geohash => { | |
| if (!GEOHASH_RE.test(v)) throw new TypeError('Invalid geohash') | |
| return v.toLowerCase() as Geohash | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared-types/src/geo.ts` at line 14, The asGeohash function
currently blindly casts any string to Geohash; update asGeohash to validate the
input string against a geohash format (e.g., allowed base32 chars and length
constraints) before branding: if the input matches the geohash regex allow the
cast (return v as Geohash), otherwise throw a clear error or return a safe
failure (e.g., undefined or Result) so invalid values cannot be silently
accepted; update callers if you change the return shape. Ensure references to
asGeohash and the Geohash type are the touchpoints for this change.
Comment on lines
+24
to
+43
| function canonicalize(value: unknown): unknown { | ||
| if (value === null || typeof value !== 'object') { | ||
| return value | ||
| } | ||
| if (Array.isArray(value)) { | ||
| return value.map(canonicalize) | ||
| } | ||
| // Reject non-plain objects to prevent silent hash collisions. | ||
| // Map, Set, and RegExp all return [] from Object.keys() and would | ||
| // produce the same hash as {}, making them undetectable failures. | ||
| if (value instanceof Map || value instanceof Set || value instanceof RegExp) { | ||
| throw new TypeError(`canonicalPayloadHash does not support ${value.constructor.name}`) | ||
| } | ||
| const record = value as Record<string, unknown> | ||
| const sortedKeys = Object.keys(record).sort() | ||
| const result: Record<string, unknown> = {} | ||
| for (const key of sortedKeys) { | ||
| result[key] = canonicalize(record[key]) | ||
| } | ||
| return result |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider handling Date objects explicitly for clarity.
The current implementation relies on JSON.stringify's default behavior for Date objects (converting to ISO string), which is deterministic and works correctly. However, for defensive completeness, you might consider handling Date explicitly—either by converting to ISO string or throwing, to make the behavior documented and intentional.
Other exotic objects like Error, Promise, or functions would serialize to {} or undefined, potentially causing silent hash collisions. These are unlikely in API payloads, so this is low priority.
♻️ Optional: Explicit Date handling
if (value instanceof Map || value instanceof Set || value instanceof RegExp) {
throw new TypeError(`canonicalPayloadHash does not support ${value.constructor.name}`)
}
+ if (value instanceof Date) {
+ return value.toISOString()
+ }
const record = value as Record<string, unknown>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function canonicalize(value: unknown): unknown { | |
| if (value === null || typeof value !== 'object') { | |
| return value | |
| } | |
| if (Array.isArray(value)) { | |
| return value.map(canonicalize) | |
| } | |
| // Reject non-plain objects to prevent silent hash collisions. | |
| // Map, Set, and RegExp all return [] from Object.keys() and would | |
| // produce the same hash as {}, making them undetectable failures. | |
| if (value instanceof Map || value instanceof Set || value instanceof RegExp) { | |
| throw new TypeError(`canonicalPayloadHash does not support ${value.constructor.name}`) | |
| } | |
| const record = value as Record<string, unknown> | |
| const sortedKeys = Object.keys(record).sort() | |
| const result: Record<string, unknown> = {} | |
| for (const key of sortedKeys) { | |
| result[key] = canonicalize(record[key]) | |
| } | |
| return result | |
| function canonicalize(value: unknown): unknown { | |
| if (value === null || typeof value !== 'object') { | |
| return value | |
| } | |
| if (Array.isArray(value)) { | |
| return value.map(canonicalize) | |
| } | |
| // Reject non-plain objects to prevent silent hash collisions. | |
| // Map, Set, and RegExp all return [] from Object.keys() and would | |
| // produce the same hash as {}, making them undetectable failures. | |
| if (value instanceof Map || value instanceof Set || value instanceof RegExp) { | |
| throw new TypeError(`canonicalPayloadHash does not support ${value.constructor.name}`) | |
| } | |
| if (value instanceof Date) { | |
| return value.toISOString() | |
| } | |
| const record = value as Record<string, unknown> | |
| const sortedKeys = Object.keys(record).sort() | |
| const result: Record<string, unknown> = {} | |
| for (const key of sortedKeys) { | |
| result[key] = canonicalize(record[key]) | |
| } | |
| return result |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared-validators/src/idempotency.ts` around lines 24 - 43, Update
the canonicalize function to handle Date objects explicitly: inside
canonicalize, detect value instanceof Date and return value.toISOString() (so
dates canonicalize deterministically) instead of falling through to object
handling; keep the existing Map/Set/RegExp rejection behavior and leave other
exotic types unchanged for now. This change should be applied in the
canonicalize function to ensure Date values are intentionally and
deterministically serialized when computing payload hashes.
| "noEmit": true, | ||
| "allowJs": true | ||
| }, | ||
| "include": ["eslint.config.js", "vitest.config.ts"] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Root Vitest config files =="
fd -HI '^(vitest\.config\.ts|vitest\.workspace\.ts)$' .
echo
echo "== tsconfig.eslint.json include =="
python - <<'PY'
import json
with open("tsconfig.eslint.json", "r", encoding="utf-8") as f:
data = json.load(f)
print(data.get("include", []))
PYRepository: Exc1D/bantayog-alert
Length of output: 318
Include root Vitest workspace config in ESLint's TypeScript compilation.
The root vitest.workspace.ts file exists but is missing from line 11's include list, leaving it untyped by ESLint. The current entry vitest.config.ts references a file that doesn't exist at root.
Suggested fix
- "include": ["eslint.config.js", "vitest.config.ts"]
+ "include": ["eslint.config.js", "vitest.config.ts", "vitest.workspace.ts"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "include": ["eslint.config.js", "vitest.config.ts"] | |
| "include": ["eslint.config.js", "vitest.config.ts", "vitest.workspace.ts"] |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.eslint.json` at line 11, The tsconfig used by ESLint includes an
incorrect/nonexistent root entry "vitest.config.ts" so the actual root Vitest
workspace file (vitest.workspace.ts) isn't compiled; update the "include" array
in tsconfig.eslint.json to remove "vitest.config.ts" and add
"vitest.workspace.ts" (keeping "eslint.config.js") so ESLint types cover the
root Vitest workspace file.
Comment on lines
+26
to
+30
| "test:rules": { | ||
| "dependsOn": [], | ||
| "outputs": [], | ||
| "env": ["FIRESTORE_EMULATOR_HOST"] | ||
| }, |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding inputs for the test:rules task.
The test:rules task currently has no inputs defined, which means Turbo cannot determine cache invalidation criteria. While emulator-based tests may intentionally skip caching, explicitly defining inputs (e.g., firestore.rules, database.rules.json, and any test files) would enable caching for rule validation when rules haven't changed.
"test:rules": {
"dependsOn": [],
"outputs": [],
+ "inputs": ["firestore.rules", "database.rules.json", "storage.rules", "test/**/*.rules.test.*"],
"env": ["FIRESTORE_EMULATOR_HOST"]
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "test:rules": { | |
| "dependsOn": [], | |
| "outputs": [], | |
| "env": ["FIRESTORE_EMULATOR_HOST"] | |
| }, | |
| "test:rules": { | |
| "dependsOn": [], | |
| "outputs": [], | |
| "inputs": ["firestore.rules", "database.rules.json", "storage.rules", "test/**/*.rules.test.*"], | |
| "env": ["FIRESTORE_EMULATOR_HOST"] | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@turbo.json` around lines 26 - 30, The test:rules Turbo task currently lacks
an inputs array, so add an "inputs" entry to the "test:rules" task in turbo.json
(the task named "test:rules") that lists the rule files and test sources to
allow proper cache invalidation; for example include filenames like
"firestore.rules", "database.rules.json", any rules folder (e.g., "rules/**"),
and the test harness/spec files (e.g., "test/**" or "tests/**") so Turbo can
cache/invalidates runs when those files change.
This was referenced Apr 17, 2026
Merged
4 tasks
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
2 participants
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
Test Plan
pnpm install --frozen-lockfilesucceedspnpm lint→ 0 errorspnpm format:check→ 0 issuespnpm typecheck→ 0 errorspnpm test→ 9 passing (canonicalPayloadHash)pnpm build→ all apps + packages + functionsfirebase emulators:start --only firestoreboots with rules loadedterraform init -backend=false && terraform validatepasses all 3 envsSummary by CodeRabbit
New Features
Chores