-
Notifications
You must be signed in to change notification settings - Fork 0
feat(phase-1): add identity spine infrastructure #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 11 commits
da19300
55e1c50
7234328
dbf5d3f
a59bc79
4ed9a84
73127f8
5ef7aeb
a1b52c2
18036b7
7f02941
5338ac6
b68de5c
1b7b404
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import '@testing-library/jest-dom' | ||
| import { describe, it, expect } from 'vitest' | ||
| import { render, screen } from '@testing-library/react' | ||
| import { vi } from 'vitest' | ||
|
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' }, | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| 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() | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,53 @@ | ||
| 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.title}-${String(alert.publishedAt)}`} className={styles.alert}> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use a stable unique alert key instead of title+timestamp concatenation. Line 43 can collide when two alerts share the same title and Proposed direction-<article key={`${alert.title}-${String(alert.publishedAt)}`} className={styles.alert}>
+<article key={alert.id} className={styles.alert}>(Requires 🤖 Prompt for AI Agents |
||
| <h2>{alert.title}</h2> | ||
| <p>{alert.body}</p> | ||
| <span>{alert.severity}</span> | ||
| </article> | ||
| ))} | ||
| </div> | ||
| </section> | ||
| </main> | ||
| ) | ||
| } | ||
| 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 })) | ||
| } | ||
| }) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Comment on lines
+95
to
+117
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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" -C2Repository: 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"
fiRepository: Exc1D/bantayog-alert Length of output: 1588 Avoid creating Firestore subscriptions after unmount. The ♻️ 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 |
||
| }) | ||
| .catch((error: unknown) => { | ||
| if (!unmounted.current) { | ||
| setState((current) => ({ | ||
| ...current, | ||
| status: 'error', | ||
| error: error instanceof Error ? error.message : 'Pseudonymous sign-in failed', | ||
| })) | ||
| } | ||
| }) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| return () => { | ||
| unmounted.current = true | ||
| cleanup.current.stopAlerts() | ||
| cleanup.current.stopVersion() | ||
| } | ||
| }, []) | ||
|
|
||
| return state | ||
| } | ||
| 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'], | ||
| }, | ||
| }) |
There was a problem hiding this comment.
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