fix(db-scripts): refuse to run unscoped, surface 0-row writes#490
fix(db-scripts): refuse to run unscoped, surface 0-row writes#490
Conversation
✅ Deploy Preview for agent-native-scheduling ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-meeting-notes ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-voice ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for agent-native-design ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This comment has been minimized.
This comment has been minimized.
|
Review Agent skipped this PR — @zuchka doesn't have a Builder seat in this space. If you're @zuchka: you may already have a seat under a different GitHub account. Reconnect GitHub If you're an admin: Add @zuchka to this space |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@builderio-bot please address these things Should fix
Nits
Praise
|
1 similar comment
|
@builderio-bot please address these things Should fix
Nits
Praise
|
|
All changes are committed. Here's a summary of what was done to address each piece of feedback: Should fix
Nits addressed
The two "skip-able" nits ( |
|
@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable) |
|
Addressed all items from the review: Should fix
Nits
PR body unchecked box: Checked. CI status: Added |
❌ Deploy Preview for agent-native-starter failed.
|
❌ Deploy Preview for nutritrack-daily-calories failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-content failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-dispatch failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-forms failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-calendar failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-macros failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-mail failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-slides failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-issues failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-recruiting failed. Why did it fail? →
|
❌ Deploy Preview for agent-native-videos failed. Why did it fail? →
|
|
No new human feedback for 20+ minutes. Here's the final state: CI status:
Review feedback addressed:
|
|
@builderio-bot CI failing please fix |
|
@builderio-bot a rebase off latest main might do it |
|
The scaffold E2E was failing because the CI merge commit includes main's Fixed in the latest commit:
The old |
|
I encountered an error while processing your request. Please try again. |
❌ Deploy Preview for agent-native-fw failed. Why did it fail? →
|
|
I encountered an error while processing your request. Please try again. |
|
@builderio-bot CI failing please fix |
|
Two root causes fixed and pushed:
CI should re-run and the Netlify builds should now succeed since |
|
@builderio-bot look at latest PR feedback and fix anything you agree with. Be skeptical. Reply to every comment (directly on the comment thread of each comment) if you fixed it or not and why. then check back every 2 minutes on a loop and see if any new feedback posted, until at least 20 minutes go by without any new feedback posted we want to address, including making sure CI passes too and no merge conflicts (make sure code is mergeable) |
|
No new reviews or comments. The PR remains green and mergeable, awaiting reviewer approval. I'll keep watching for the next 20 minutes without new feedback before concluding. |
|
@builderio-bot fix the cloudflare issues example one: 16:20:42.891 | ✓ built in 6.52s |
|
All good:
The Cloudflare fix is in. The root cause was that The only remaining blocker is |
Previously, db-exec / db-query / db-patch silently fell back to unscoped mode when neither a request context nor AGENT_USER_EMAIL was set. INSERTs landed with the migration default owner_email='local@localhost' (invisible to the UI session user) and UPDATE/DELETE ran cross-tenant against every user's rows. Changes: - scoping.ts: getUserEmail() throws on null/empty as well as the local@localhost sentinel; both buildScoping* functions drop the inactive short-circuit. Error message names AGENT_USER_EMAIL. - exec.ts: when a write returns 0 rows changed, append a hint that the WHERE clause may not match a row visible to the current user (mirrors the wording db-patch already uses). - scoping.spec.ts / parameterized.spec.ts: tests follow the new contract; three parameterized tests now stub AGENT_USER_EMAIL. - templates/slides/.gitignore: ignore stray app.db / local.db at the template root so a misconfigured cwd or --db override can't litter the working tree. Deferred follow-ups (separate PRs): auto-load dev session in the CLI, drop DEFAULT 'local@localhost' from template migrations, add a CI guard, document the three access paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After changes-53 (`fix(db-scripts): refuse to run unscoped, …`), every
`pnpm action db-…` invocation threw "require an authenticated user
identity" unless the operator manually exported AGENT_USER_EMAIL. The
commit message flagged this as a deferred follow-up; this is it.
- New `scripts/dev-session.ts` with `resolveDevUserEmail()`. Returns
`AGENT_USER_EMAIL` if explicitly set, otherwise reads
`SELECT email FROM sessions ORDER BY created_at DESC LIMIT 1` —
mirroring the A2A receiver fallback in `agent-chat-plugin.ts`.
Strict gating: `NODE_ENV !== "production"`, `AUTH_MODE` unset or
`local`, sentinel `local@localhost` filtered at the SQL level.
- `scripts/runner.ts` extracts dispatch into `dispatchAction()` and
wraps it in `runWithRequestContext({ userEmail, orgId })`. One
injection point covers both local-action and core-script branches.
Uses request-context (not env mutation) per the warning in
`server/request-context.ts`.
- `scripts/db/scoping.ts` error message now leads with "open the app
and sign in" — keeps the existing "require an authenticated user
identity" substring that scoping.spec.ts asserts on.
- New `dev-session.spec.ts` covers env pass-through, prod refusal,
AUTH_MODE gate, dev success, empty/missing sessions table, blank
email handling.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One-shot fix for local DBs that accumulated rows owned by the dev sentinel `local@localhost`. Pre-changes-53, the scoping wrapper silently fell back to that owner when no real identity was present, so data created via CLI runs (or older runner versions) landed under the sentinel and is now invisible to the actual signed-in user. Discovers every table with an `owner_email` column and reassigns `local@localhost` rows to the email passed via `--to`. Refuses on `NODE_ENV=production`; refuses against Postgres unless `AN_ALLOW_PG_DEV_OWNER_RESET=1` is explicitly set (so a misconfigured DATABASE_URL can't sweep production data into one tenant). Usage: pnpm action db-reset-dev-owner --to me@example.com --dry-run pnpm action db-reset-dev-owner --to me@example.com pnpm action db-reset-dev-owner --to me@example.com --table decks Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 participants
The bug
The in-app agent's
db-exectool wrote rows that the UI couldn't see, while shellsqlite3 data/app.dbwrites did show up. The user's instinct ("they hit different databases / processes") wasn't quite right — the actual cause is more dangerous:getUserEmail()in `packages/core/src/scripts/db/scoping.ts` returned `null` when neither a request context nor `AGENT_USER_EMAIL` was set. It only threw on the explicit `local@localhost` sentinel.This affects any path where db-exec / db-query / db-patch run without identity in scope — most importantly, `pnpm action db-exec ...` invoked via the dev-mode `shell` tool, which spawns a child process with no inherited `runWithRequestContext`.
What this PR does
Three tightly-scoped changes (the "fail loud, hint clearly, prevent regression" set):
Out of scope (deliberately, for follow-ups)
Test plan
🤖 Generated with Claude Code