feat: add basic auth to todos, migrate examples to Sync() lifecycle#39
feat: add basic auth to todos, migrate examples to Sync() lifecycle#39
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the LiveTemplate examples to align with new core defaults and lifecycle patterns, while adding Basic Auth + per-user data isolation to the todos example (including cross-tab syncing via Sync()).
Changes:
- Add Basic Auth to
todos, introduceuser_idcolumn, and scope all todo queries by authenticated user. - Migrate
todosandshared-notepadto use theSync()lifecycle for cross-tab state refresh. - Remove
WithStatePersistence()usage across examples (persistence now default in core).
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ws-disabled/main.go | Removes explicit state persistence option. |
| todos/todos.tmpl | Displays logged-in username in the UI header. |
| todos/todos_test.go | Updates E2E + WebSocket tests to send Basic Auth credentials. |
| todos/state.go | Adds Username to state for UI display. |
| todos/main.go | Adds Basic Auth authenticator and wires it into template options. |
| todos/db/schema.sql | Adds user_id column and index to the todos schema. |
| todos/db/queries.sql | Scopes all SQL queries by user_id. |
| todos/db/queries.sql.go | Regenerates sqlc output to include user_id parameters/fields. |
| todos/db/models.go | Adds UserID to the generated Todo model. |
| todos/db_manager.go | Updates runtime schema creation to include user_id. |
| todos/controller.go | Uses ctx.UserID() for per-user DB access and adds Sync() lifecycle. |
| todos-progressive/main.go | Removes explicit state persistence option. |
| todos-components/main.go | Removes explicit state persistence option. |
| shared-notepad/notepad.tmpl | Updates documentation text to reference Sync(). |
| shared-notepad/main.go | Replaces WithSharedState() with an in-memory per-user store + Sync(). |
| progressive-enhancement/main.go | Removes explicit state persistence option. |
| profile-progressive/main.go | Removes explicit state persistence option. |
| login/main.go | Removes explicit state persistence option. |
| live-preview/main.go | Removes explicit state persistence option. |
| flash-messages/main.go | Removes explicit state persistence option. |
| counter/main.go | Removes explicit state persistence option. |
| avatar-upload/main.go | Removes explicit state persistence option. |
| go.mod | Updates local replace to a different worktree path. |
Comments suppressed due to low confidence (1)
todos/db_manager.go:54
runMigrationsusesCREATE TABLE IF NOT EXISTS todos (...)to introduce the newuser_idcolumn, but this won’t upgrade an existing on-disktodos.dbcreated by earlier versions (SQLite will keep the old table definition). As a result, the app will fail at runtime withno such column: user_idwhen queries run against an existing DB. Consider adding a real migration path (e.g., detect missing column viaPRAGMA table_info(todos)andALTER TABLE ... ADD COLUMN user_id ...with a sensible default / backfill strategy, or explicitly recreate the table when not in TEST_MODE).
// runMigrations creates the database schema
func runMigrations(db *sql.DB) error {
schema := `
CREATE TABLE IF NOT EXISTS todos (
id TEXT PRIMARY KEY,
user_id TEXT NOT NULL,
text TEXT NOT NULL,
completed BOOLEAN NOT NULL DEFAULT 0,
created_at DATETIME NOT NULL
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
go.mod
Outdated
| ) | ||
|
|
||
| replace github.com/livetemplate/livetemplate => ../livetemplate/.worktrees/per-connection-persist | ||
| replace github.com/livetemplate/livetemplate => ../../../livetemplate/.worktrees/auth-sync |
There was a problem hiding this comment.
The replace github.com/livetemplate/livetemplate => ../../../livetemplate/.worktrees/auth-sync directive is a local workstation path that will break builds/tests for anyone who doesn’t have that directory layout (including CI). Please remove this replace from the PR (or gate it behind a local-only workflow like a separate go.work / developer setup instructions).
| return c.loadTodos(context.Background(), state, ctx.UserID()) | ||
| } | ||
|
|
||
| func (c *TodoController) Sync(state TodoState, ctx *livetemplate.Context) (TodoState, error) { |
There was a problem hiding this comment.
Sync reloads todos for the current user, but it doesn’t re-assert state.Username = ctx.UserID() like Mount/OnConnect do. If a client ever syncs from a state snapshot that has Username unset/stale, the UI header can lose the logged-in banner. Setting state.Username inside Sync keeps the view consistent and makes Sync idempotent.
| func (c *TodoController) Sync(state TodoState, ctx *livetemplate.Context) (TodoState, error) { | |
| func (c *TodoController) Sync(state TodoState, ctx *livetemplate.Context) (TodoState, error) { | |
| state.Username = ctx.UserID() |
| type NotepadController struct { | ||
| mu sync.RWMutex | ||
| notes map[string]NotepadState // userID -> latest state | ||
| } |
There was a problem hiding this comment.
The controller stores per-user notes in an unbounded in-memory map (notes map[string]NotepadState). For a long-running server or many distinct usernames, this will grow without limit and can lead to memory pressure. Consider adding a simple eviction policy/TTL, limiting accepted usernames in the demo, or documenting that this is intentionally in-memory/demo-only.
| func (c *NotepadController) Sync(state NotepadState, ctx *livetemplate.Context) (NotepadState, error) { | ||
| c.mu.RLock() | ||
| if saved, ok := c.notes[ctx.UserID()]; ok { | ||
| state.Content = saved.Content | ||
| state.CharCount = saved.CharCount | ||
| state.SavedAt = saved.SavedAt | ||
| } | ||
| c.mu.RUnlock() | ||
| return state, nil |
There was a problem hiding this comment.
Sync restores Content/CharCount/SavedAt but never sets state.Username = ctx.UserID(). If a sync happens on a state instance where Username is empty (or if future refactors call Sync before Mount), the template’s “Logged in as …” line can become blank. Setting Username in Sync makes the lifecycle handler self-contained.
| err := chromedp.Run(ctx, | ||
| chromedp.Navigate(e2etest.GetChromeTestURL(serverPort)), | ||
| chromedp.Navigate(fmt.Sprintf("http://alice:password@host.docker.internal:%d/", serverPort)), | ||
| e2etest.WaitForWebSocketReady(5*time.Second), // Wait for WebSocket init and first update |
There was a problem hiding this comment.
The E2E test now embeds credentials in the navigation URL (http://alice:password@host.docker.internal...). Chrome/user agents treat user:pass@ URLs inconsistently and it also leaks credentials into logs and debugging output. A more robust approach is to keep using e2etest.GetChromeTestURL(serverPort) and set the Authorization header via CDP (network.SetExtraHTTPHeaders) before Navigate, similar to other Docker Chrome tests in this repo.
|
|
||
| for i := 0; i < 50; i++ { // 10 seconds max (50 * 200ms) | ||
| resp, err := http.Get(serverURL) | ||
| req, _ := http.NewRequest("GET", serverURL, nil) |
There was a problem hiding this comment.
The test drops the error return from http.NewRequest (req, _ := ...). While the URL is currently well-formed, ignoring the error can mask future regressions and can lead to a nil request being used. Please handle the error (fail the test) to keep the readiness check reliable.
| req, _ := http.NewRequest("GET", serverURL, nil) | |
| req, err := http.NewRequest("GET", serverURL, nil) | |
| if err != nil { | |
| t.Fatalf("failed to create readiness request: %v", err) | |
| } |
…#297) Todos example: - Add BasicAuthenticator with demo users (alice/password, bob/password) - Add user_id column to DB for per-user todo isolation - Add Sync() lifecycle for automatic cross-tab sync - Add OnConnect() for fresh data on reconnect - Update tests with auth headers Shared-notepad: - Replace WithSharedState() with Sync() lifecycle + in-memory store All examples: - Remove WithStatePersistence() (persistence is now always on) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f624c6a to
48a177a
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
BasicAuthenticatorwith per-user DB isolation andSync()for cross-tab syncWithSharedState()withSync()lifecycle + in-memory storeWithStatePersistence()(persistence is now default in core)Depends on: livetemplate/livetemplate#298
Test plan
go build ./...🤖 Generated with Claude Code