Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,25 @@ pnpm build # Build all apps and packages
pnpm emulators # Start Firebase emulator suite
```

## Citizen PWA env vars

Set these in `apps/citizen-pwa/.env.local` for local development:

- `VITE_FIREBASE_API_KEY`
- `VITE_FIREBASE_AUTH_DOMAIN`
- `VITE_FIREBASE_PROJECT_ID`
- `VITE_FIREBASE_APP_ID`
- `VITE_FIREBASE_MESSAGING_SENDER_ID`
- `VITE_FIREBASE_STORAGE_BUCKET`
- `VITE_FIREBASE_APP_CHECK_SITE_KEY`

## Phase 1 verification

- `pnpm test`
- `pnpm --filter @bantayog/functions test:unit`
- `pnpm --filter @bantayog/functions test:rules`
- `pnpm lint && pnpm typecheck && pnpm build`

Comment on lines +47 to +53
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add an explicit staging-approval step to this verification checklist.

Given this PR touches auth/function/security surface, include a final checklist item that requires explicit staging approval after emulator + full-suite validation.

Based on learnings: For changes to security rules, DB indexes, deployment config, auth flows, or Cloud Functions with existing traffic: show the diff, deploy to dev emulator first, run full test suite, then request explicit staging approval.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 47 - 53, Update the "Phase 1 verification" checklist
to add a final explicit "staging approval" step: after running the listed
commands (pnpm test, pnpm --filter `@bantayog/functions` test:unit, pnpm --filter
`@bantayog/functions` test:rules, lint/typecheck/build) require a documented
staging-approval item that mandates deploying changes to the dev emulator,
running the full test suite, and obtaining explicit staging approval before any
production deploy for changes touching auth, security rules, DB indexes,
deployment config, auth flows, or Cloud Functions; reference the "Phase 1
verification" header and add a bullet that outlines these exact requirements.

## Repository layout

See `docs/superpowers/specs/2026-04-17-phase-0-design.md` §1 for the canonical layout.
Expand Down
6 changes: 5 additions & 1 deletion apps/citizen-pwa/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,19 @@
"build": "tsc --noEmit && vite build",
"preview": "vite preview",
"lint": "eslint src",
"typecheck": "tsc --noEmit"
"typecheck": "tsc --noEmit",
"test": "vitest run"
},
"dependencies": {
"@bantayog/shared-firebase": "workspace:*",
"@bantayog/shared-types": "workspace:*",
"@bantayog/shared-ui": "workspace:*",
"react": "^19.2.5",
"react-dom": "^19.2.5"
},
"devDependencies": {
"@testing-library/jest-dom": "^6.4.0",
"@testing-library/react": "^16.0.0",
"@types/react": "^19.2.14",
"@types/react-dom": "^19.2.3",
"@vitejs/plugin-react": "^6.0.1",
Expand Down
65 changes: 50 additions & 15 deletions apps/citizen-pwa/src/App.module.css
Original file line number Diff line number Diff line change
@@ -1,25 +1,60 @@
@import '@bantayog/shared-ui/theme.css';

.container {
.page {
min-height: 100vh;
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
display: grid;
place-items: center;
padding: var(--space-8);
background: var(--color-bg);
color: var(--color-text);
font-family: var(--font-sans);
background:
radial-gradient(circle at top left, rgb(20 184 166 / 0.16), transparent 28rem),
linear-gradient(180deg, #031521 0%, #0a2230 100%);
}

.heading {
font-size: var(--font-size-lg);
color: var(--color-accent);
margin: 0 0 var(--space-2) 0;
.panel {
width: min(48rem, 100%);
padding: var(--space-8);
border-radius: 1.5rem;
background: rgb(255 255 255 / 0.92);
color: #092033;
box-shadow: 0 24px 80px rgb(0 0 0 / 0.18);
}

.eyebrow {
margin: 0 0 var(--space-2);
text-transform: uppercase;
letter-spacing: 0.12em;
font-size: 0.75rem;
color: #0f766e;
}

.subheading {
font-size: var(--font-size-md);
color: var(--color-text-muted);
.title {
margin: 0;
font-size: clamp(2rem, 5vw, 3rem);
}

.summary {
margin: var(--space-3) 0 var(--space-6);
color: #345064;
}

.meta {
display: grid;
grid-template-columns: repeat(auto-fit, minmax(10rem, 1fr));
gap: var(--space-4);
}

.feed {
margin-top: var(--space-6);
display: grid;
gap: var(--space-4);
}

.alert {
padding: var(--space-4);
border-radius: 1rem;
background: #ecfeff;
}

.error {
color: #b91c1c;
}
41 changes: 41 additions & 0 deletions apps/citizen-pwa/src/App.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import '@testing-library/jest-dom'
import { describe, it, expect } from 'vitest'
import { render, screen } from '@testing-library/react'
import { vi } from 'vitest'
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
import { App } from './App.js'

vi.mock('./useCitizenShell.js', () => ({
useCitizenShell: () => ({
status: 'ready',
authState: 'signed-in',
appCheckState: 'active',
user: { uid: 'anon-123' },
minAppVersion: {
citizen: '0.1.0',
admin: '0.1.0',
responder: '0.1.0',
updatedAt: 1713350400000,
},
alerts: [
{
title: 'System online',
body: 'Citizen shell wired for Phase 1.',
severity: 'info',
publishedAt: 1713350400000,
publishedBy: 'phase-1-bootstrap',
},
],
error: null,
}),
}))

describe('App', () => {
it('renders auth status, app version, and the hello-world alert feed', () => {
render(<App />)

expect(screen.getByText(/anon-123/)).toBeInTheDocument()
expect(screen.getByText(/System online/)).toBeInTheDocument()
expect(screen.getByText(/0.1.0/)).toBeInTheDocument()
expect(screen.getByText(/signed-in/)).toBeInTheDocument()
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
})
})
52 changes: 49 additions & 3 deletions apps/citizen-pwa/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,56 @@
import styles from './App.module.css'
import { useCitizenShell } from './useCitizenShell.js'

export function App() {
const state = useCitizenShell()

return (
<main className={styles.container}>
<h1 className={styles.heading}>Bantayog Alert — Citizen</h1>
<p className={styles.subheading}>Phase 0 scaffolding. Reporting arrives in Phase 2.</p>
<main className={styles.page}>
<section className={styles.panel}>
<p className={styles.eyebrow}>Bantayog Alert</p>
<h1 className={styles.title}>Citizen Phase 1 shell</h1>
<p className={styles.summary}>
Pseudonymous sign-in, app health, and a hello-world alert feed.
</p>

<dl className={styles.meta}>
<div>
<dt>Status</dt>
<dd>{state.status}</dd>
</div>
<div>
<dt>Auth</dt>
<dd>{state.authState}</dd>
</div>
<div>
<dt>App Check</dt>
<dd>{state.appCheckState}</dd>
</div>
<div>
<dt>User UID</dt>
<dd>{state.user?.uid ?? 'unavailable'}</dd>
</div>
<div>
<dt>Minimum citizen version</dt>
<dd>{state.minAppVersion?.citizen ?? 'unavailable'}</dd>
</div>
</dl>

{state.error ? <p className={styles.error}>{state.error}</p> : null}

<div className={styles.feed}>
{state.alerts.map((alert) => (
<article
key={`${alert.publishedBy}-${String(alert.publishedAt)}`}
className={styles.alert}
>
<h2>{alert.title}</h2>
<p>{alert.body}</p>
<span>{alert.severity}</span>
</article>
))}
</div>
</section>
</main>
)
}
137 changes: 137 additions & 0 deletions apps/citizen-pwa/src/useCitizenShell.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { useEffect, useState, useRef } from 'react'
import {
createAppCheck,
createFirebaseWebApp,
ensurePseudonymousSignIn,
getFirebaseAuth,
getFirebaseDb,
parseFirebaseWebEnv,
subscribeAlerts,
subscribeMinAppVersion,
} from '@bantayog/shared-firebase'
import type { AlertDoc, MinAppVersionDoc } from '@bantayog/shared-types'

interface ShellState {
status: 'booting' | 'ready' | 'error'
authState: 'signed-out' | 'signed-in'
appCheckState: 'pending' | 'active' | 'failed'
user: { uid: string } | null
minAppVersion: MinAppVersionDoc | null
alerts: AlertDoc[]
error: string | null
}

const initialState: ShellState = {
status: 'booting',
authState: 'signed-out',
appCheckState: 'pending',
user: null,
minAppVersion: null,
alerts: [],
error: null,
}

export function useCitizenShell(): ShellState {
const [state, setState] = useState<ShellState>(initialState)

// Guard against state updates on unmounted component
const unmountedRef = useRef(false)

// Cleanup ref holds unsubscribe functions; initialized as no-ops in case
// unmount happens before subscriptions are established (Firestore onSnapshot
// returns unsubscribe only after subscription is created)
const cleanupRef = useRef<{ stopAlerts: () => void; stopVersion: () => void }>({
// eslint-disable-next-line @typescript-eslint/no-empty-function
stopAlerts: () => {},
// eslint-disable-next-line @typescript-eslint/no-empty-function
stopVersion: () => {},
})

useEffect(() => {
unmountedRef.current = false

let env
let app
let db
let auth

try {
env = parseFirebaseWebEnv(import.meta.env)
app = createFirebaseWebApp(env)
db = getFirebaseDb(app)
auth = getFirebaseAuth(app)
} catch (error) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (!unmountedRef.current) {
setState({
...initialState,
status: 'error',
appCheckState: 'failed',
error: error instanceof Error ? error.message : 'Firebase initialization failed',
})
}
return
}

// Capture refs in local variables to avoid exhaustive-deps warning in cleanup
const unmounted = unmountedRef
const cleanup = cleanupRef

try {
createAppCheck(app, env)
if (!unmounted.current) {
setState((current) => ({ ...current, appCheckState: 'active' }))
}
} catch (error) {
if (!unmounted.current) {
setState((current) => ({
...current,
appCheckState: 'failed',
error: error instanceof Error ? error.message : 'App Check initialization failed',
}))
}
}

void ensurePseudonymousSignIn(auth)
.then((user) => {
// Set auth state once immediately after sign-in
if (!unmounted.current) {
setState((current) => ({
...current,
status: 'ready',
authState: 'signed-in',
user: { uid: user.uid },
}))
}

cleanup.current.stopVersion = subscribeMinAppVersion(db, (minAppVersion) => {
if (!unmounted.current) {
setState((current) => ({ ...current, minAppVersion }))
}
})

cleanup.current.stopAlerts = subscribeAlerts(db, (alerts) => {
if (!unmounted.current) {
setState((current) => ({ ...current, alerts }))
}
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +95 to +117
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="apps/citizen-pwa/src/useCitizenShell.ts"

echo "Inspecting sign-in then-block and subscription registration:"
nl -ba "$file" | sed -n '95,120p'

echo
echo "Expected after fix: early guard 'if (unmounted.current) return' before subscribe* calls."
rg -n "if \\(unmounted\\.current\\) return|subscribeMinAppVersion|subscribeAlerts" "$file" -C2

Repository: Exc1D/bantayog-alert

Length of output: 184


🏁 Script executed:

#!/bin/bash
file="apps/citizen-pwa/src/useCitizenShell.ts"

# Check file exists and get line count
if [ -f "$file" ]; then
  total_lines=$(wc -l < "$file")
  echo "File found. Total lines: $total_lines"
  echo "---"
  
  # Read lines 90-130 to see context around 95-117
  sed -n '90,130p' "$file" | cat -n
else
  echo "File not found"
fi

Repository: Exc1D/bantayog-alert

Length of output: 1588


Avoid creating Firestore subscriptions after unmount.

The .then() handler executes unconditionally when the sign-in promise resolves, even if the component has unmounted. The if (!unmounted.current) guards on lines 99 and 105 only protect the setState calls inside subscription callbacks, not the subscription registration itself. This causes subscribeMinAppVersion() and subscribeAlerts() to create listeners after cleanup has already run, leaking active Firestore subscriptions.

♻️ Proposed fix
 void ensurePseudonymousSignIn(auth)
   .then((user) => {
+    if (unmounted.current) return
+
     // Set auth state once immediately after sign-in
-    if (!unmounted.current) {
-      setState((current) => ({
-        ...current,
-        status: 'ready',
-        authState: 'signed-in',
-        user: { uid: user.uid },
-      }))
-    }
+    setState((current) => ({
+      ...current,
+      status: 'ready',
+      authState: 'signed-in',
+      user: { uid: user.uid },
+    }))

     cleanup.current.stopVersion = subscribeMinAppVersion(db, (minAppVersion) => {
       if (!unmounted.current) {
         setState((current) => ({ ...current, minAppVersion }))
       }
     })

     cleanup.current.stopAlerts = subscribeAlerts(db, (alerts) => {
       if (!unmounted.current) {
         setState((current) => ({ ...current, alerts }))
       }
     })
   })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/citizen-pwa/src/useCitizenShell.ts` around lines 95 - 117, After
ensurePseudonymousSignIn resolves, the code currently registers Firestore
listeners unconditionally which can run after the component unmounted; update
the .then() handler in useCitizenShell so you first check unmounted.current and
return early if true (i.e., do not call setState or create subscriptions), and
only after that assign cleanup.current.stopVersion = subscribeMinAppVersion(db,
...) and cleanup.current.stopAlerts = subscribeAlerts(db, ...); ensure all
setState calls and subscription registrations remain inside the same
unmounted.current guard so subscriptions are never created after unmount
(references: ensurePseudonymousSignIn, unmounted.current,
cleanup.current.stopVersion, cleanup.current.stopAlerts, subscribeMinAppVersion,
subscribeAlerts, setState).

})
.catch((error: unknown) => {
if (!unmounted.current) {
setState((current) => ({
...current,
status: 'error',
error: error instanceof Error ? error.message : 'Pseudonymous sign-in failed',
}))
}
})
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return () => {
unmounted.current = true
cleanup.current.stopAlerts()
cleanup.current.stopVersion()
}
}, [])

return state
}
2 changes: 1 addition & 1 deletion apps/citizen-pwa/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"rootDir": "src",
"types": ["vite/client"],
"types": ["vite/client", "vitest/globals"],
"noEmit": true
},
"include": ["src/**/*"]
Expand Down
10 changes: 10 additions & 0 deletions apps/citizen-pwa/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { defineConfig } from 'vitest/config'

export default defineConfig({
test: {
globals: true,
environment: 'happy-dom',
include: ['src/**/*.test.tsx'],
setupFiles: ['@testing-library/jest-dom/vitest'],
},
})
Loading
Loading