From 9f0275f1790d41e0138d51f53371bfb37b3f1b73 Mon Sep 17 00:00:00 2001 From: MikeyZhang75 Date: Sun, 6 Sep 2026 10:16:23 +0800 Subject: [PATCH 1/7] fix(example): bound projects.list reads to the page, not the table `projects.list` asked "owner OR member" of `projects`, which no index spans, so it walked the table from the newest end and post-filtered in JS. A viewer with nothing to see drained the whole table, and every scanned row joined the subscription read set. Measured 40 scanned / 0 returned at 40 rows. Two more defects were hiding behind it: - `.filter()` hands the callback the raw Convex row, whose id field is `_id`, so `memberProjectIds.has(project.id)` was `has(undefined)` and always false. Projects visible only through a membership never appeared in the list at all. - `hasAnyProject` was unbounded too: `{ ownerId, archived }` ties `index('archived')` and `index('ownerId')` at the same planner score, declaration order wins, and the loser becomes a post-filter over the whole partition. 40 and 43 reads against bounds of 2 and 3. Add `projectAccess`, one row per (user, project), indexed `(userId, archived, projectCreatedAt)`. `_helpers/project_access.ts` owns every grant/revoke and is the single owner of "which projects can this user see", used by `list`, `listForDropdown` and `hasAny`. 4 reads at 10 noise rows and 4 at 400. Maintained by explicit mutation helpers, not schema triggers: async cascade continuation workers build their ORM from the raw `ctx.db` and drop triggers past the first batch, so a trigger-maintained copy would drift. Deletion needs neither, both FKs cascade and cascades still run in those batches. `revokeProjectAccess` re-derives entitlement rather than deleting blindly, and `grantProjectAccess` derives `isOwner` from the project so a caller cannot mislabel a row. Also repair the read-bound harness, which was measuring nothing: - `countDocumentReads` patched `db.get` as a one-arg forwarder while the lifecycle writer only calls the two-arg overload, so `oldDoc`/`newDoc` came back null and every trigger was silently skipped. - The example read-bound tests installed the counter after the ORM was built, so they measured a constant 0. Fixing placement exposed the `hasAnyProject` scan and a tag-merge bound that was really 16, not 12 (same at 40 and 120 target rows, so the property it pins still holds). Bounds are asserted as equalities so a dead counter fails loudly. Add compound `isPublic_archived` for the unauthenticated branch, which had the same planner tie and read every archived public project. --- .../orm/example-project-access-reads.test.ts | 588 ++++++++++++++++++ .../example-project-existence-reads.test.ts | 156 ----- convex/orm/example-tag-merge-reads.test.ts | 56 +- convex/setup.testing.ts | 53 +- ...026-09-05-443-projects-list-read-bounds.md | 549 ++++++++++++++++ .../functions/_helpers/project_access.ts | 244 ++++++++ .../functions/_helpers/project_existence.ts | 26 - example/convex/functions/aggregateDemo.ts | 5 +- .../generated/procedure-names.gen.ts | 8 +- example/convex/functions/ormDemo.ts | 37 +- example/convex/functions/projects.ts | 124 ++-- example/convex/functions/schema.ts | 83 +++ example/convex/functions/seed.ts | 8 +- 13 files changed, 1652 insertions(+), 285 deletions(-) create mode 100644 convex/orm/example-project-access-reads.test.ts delete mode 100644 convex/orm/example-project-existence-reads.test.ts create mode 100644 docs/plans/2026-09-05-443-projects-list-read-bounds.md create mode 100644 example/convex/functions/_helpers/project_access.ts delete mode 100644 example/convex/functions/_helpers/project_existence.ts diff --git a/convex/orm/example-project-access-reads.test.ts b/convex/orm/example-project-access-reads.test.ts new file mode 100644 index 000000000..985638a0a --- /dev/null +++ b/convex/orm/example-project-access-reads.test.ts @@ -0,0 +1,588 @@ +import { eq } from 'kitcn/orm'; +import { expect, test } from 'vitest'; +import { + backfillProjectAccess, + grantProjectAccess, + hasAnyProject, + listAccessibleProjects, + revokeProjectAccess, + syncProjectArchived, +} from '../../example/convex/functions/_helpers/project_access'; +import schema, { + projectAccessTable, + projectMembersTable, + projectsTable, + userTable, +} from '../../example/convex/functions/schema'; +import { + convexTest, + countDocumentReads, + withExampleEnv, + withOrm, +} from '../setup.testing'; + +/** + * `countDocumentReads` swaps `db.query`/`db.get`, and the ORM binds both when it + * is built, so the counter has to be installed *before* `withOrm`. Installed on + * a context that already carries an ORM it counts only reads issued directly + * through `ctx.db` -- a constant zero for an ORM-only test, which makes every + * bound below pass vacuously. Snapshot after seeding so only the read under test + * is counted. + */ +const withCountedOrmCtx = async ( + run: ( + ctx: { + db: Parameters[0]['db']; + orm: ReturnType>['orm']; + }, + reads: ReturnType + ) => Promise +): Promise => { + await withExampleEnv(async () => { + const t = convexTest(schema); + await t.run(async (baseCtx) => { + const reads = countDocumentReads(baseCtx); + const ctx = withOrm(baseCtx, schema); + await run(ctx, reads); + }); + }); +}; + +type Ctx = Parameters[0]>[0]; + +type AccessRow = { + archived: boolean; + isOwner: boolean; + projectCreatedAt: number; + projectId: string; + userId: string; +}; + +let emailSeq = 0; +const makeUser = async (ctx: Ctx, label: string): Promise => { + emailSeq += 1; + const [user] = await ctx.orm + .insert(userTable) + .values({ + name: label, + email: `${label}-${emailSeq}@access.test`, + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + .returning({ id: userTable.id }); + return user.id; +}; + +/** Create a project exactly the way `projects.create` does. */ +const makeProject = async ( + ctx: Ctx, + input: { name: string; ownerId: string; archived?: boolean } +) => { + const [project] = await ctx.orm + .insert(projectsTable) + .values({ + name: input.name, + isPublic: false, + archived: input.archived ?? false, + ownerId: input.ownerId, + }) + .returning(); + + await grantProjectAccess(ctx, { project, userId: input.ownerId }); + + return project; +}; + +/** Add a member exactly the way `projects.addMember` does. */ +const addMember = async ( + ctx: Ctx, + input: { project: Awaited>; userId: string } +) => { + await ctx.orm.insert(projectMembersTable).values({ + projectId: input.project.id, + userId: input.userId, + }); + await grantProjectAccess(ctx, { + project: input.project, + userId: input.userId, + }); +}; + +const NOISE = 40; + +const seedNoise = async (ctx: Ctx, ownerId: string, count = NOISE) => { + for (let index = 0; index < count; index += 1) { + await makeProject(ctx, { name: `noise-${index}`, ownerId }); + } +}; + +/** + * `projects.list` used to ask "owner OR member" of `projects`, which no index + * spans, so it walked the table from the newest end and post-filtered in JS. + * A viewer with nothing to see drained the whole table, and every scanned row + * joined the subscription's read set. + * + * These bounds must not track table size. + */ +test('listing reads nothing proportional to projects owned by other users', async () => { + await withCountedOrmCtx(async (ctx, reads) => { + const other = await makeUser(ctx, 'other'); + const viewer = await makeUser(ctx, 'viewer'); + await seedNoise(ctx, other); + + const before = reads.scanned; + + const results = await listAccessibleProjects(ctx, { + userId: viewer, + archived: false, + cursor: null, + limit: 20, + }); + + expect(results.page).toEqual([]); + expect(results.isDone).toBe(true); + // The index range for this viewer is empty. The scan read NOISE rows here. + expect(reads.scanned - before).toBeLessThanOrEqual(2); + }); +}); + +test('listing cost tracks the page, not the table', async () => { + const measure = async (noise: number) => { + let scanned = 0; + let names: string[] = []; + + await withCountedOrmCtx(async (ctx, reads) => { + const other = await makeUser(ctx, 'other'); + const viewer = await makeUser(ctx, 'viewer'); + await seedNoise(ctx, other, noise); + + await makeProject(ctx, { name: 'owned-a', ownerId: viewer }); + const shared = await makeProject(ctx, { + name: 'shared-a', + ownerId: other, + }); + await addMember(ctx, { project: shared, userId: viewer }); + + const before = reads.scanned; + const results = await listAccessibleProjects(ctx, { + userId: viewer, + archived: false, + cursor: null, + limit: 20, + }); + scanned = reads.scanned - before; + names = results.page.map((project) => project.name); + }); + + return { names, scanned }; + }; + + const small = await measure(10); + const large = await measure(400); + + // Same visible projects, and the same cost, at 40x the table size. + expect(small.names.sort()).toEqual(['owned-a', 'shared-a']); + expect(large.names.sort()).toEqual(['owned-a', 'shared-a']); + // Exactly one access row plus its project, per visible project. Asserted as + // an equality rather than a ceiling so that a counter which silently reports + // zero -- the failure mode when it is installed after the ORM is built -- + // fails here instead of passing vacuously. + expect(small.scanned).toBe(2 * small.names.length); + expect(large.scanned).toBe(small.scanned); +}); + +/** + * The read model this replaces post-filtered on `project.id`, but `.filter()` + * receives the raw Convex row, whose id field is `_id`. `memberProjectIds.has( + * project.id)` was therefore `has(undefined)` and always false, so a project you + * could only see through a membership never appeared in the list at all. + */ +test('a project the viewer can only see through a membership is listed', async () => { + await withCountedOrmCtx(async (ctx) => { + const other = await makeUser(ctx, 'other'); + const viewer = await makeUser(ctx, 'viewer'); + await seedNoise(ctx, other, 5); + + const shared = await makeProject(ctx, { name: 'shared', ownerId: other }); + await addMember(ctx, { project: shared, userId: viewer }); + + const results = await listAccessibleProjects(ctx, { + userId: viewer, + archived: false, + cursor: null, + limit: 20, + }); + + expect(results.page.map((project) => project.name)).toEqual(['shared']); + }); +}); + +test('paging walks every visible project once, newest first', async () => { + await withCountedOrmCtx(async (ctx) => { + const other = await makeUser(ctx, 'other'); + const viewer = await makeUser(ctx, 'viewer'); + + // Interleave owned and member-only projects so a page boundary has to hold + // across both kinds. + const expected: string[] = []; + for (let index = 0; index < 9; index += 1) { + const owned = index % 2 === 0; + const name = `p-${String(index).padStart(2, '0')}`; + if (owned) { + await makeProject(ctx, { name, ownerId: viewer }); + } else { + const project = await makeProject(ctx, { name, ownerId: other }); + await addMember(ctx, { project, userId: viewer }); + } + expected.push(name); + await makeProject(ctx, { name: `noise-${index}`, ownerId: other }); + } + expected.reverse(); + + const seen: string[] = []; + let cursor: string | null = null; + let isDone = false; + let pages = 0; + + while (!isDone) { + const results = await listAccessibleProjects(ctx, { + userId: viewer, + archived: false, + cursor, + limit: 4, + }); + seen.push(...results.page.map((project) => project.name)); + cursor = results.continueCursor; + isDone = results.isDone; + pages += 1; + expect(pages).toBeLessThan(10); + } + + expect(seen).toEqual(expected); + expect(new Set(seen).size).toBe(seen.length); + }); +}); + +test('archiving moves a project between the two lists', async () => { + await withCountedOrmCtx(async (ctx) => { + const viewer = await makeUser(ctx, 'viewer'); + const other = await makeUser(ctx, 'other'); + + const project = await makeProject(ctx, { name: 'p', ownerId: viewer }); + const shared = await makeProject(ctx, { name: 'shared', ownerId: other }); + await addMember(ctx, { project: shared, userId: viewer }); + + const active = () => + listAccessibleProjects(ctx, { + userId: viewer, + archived: false, + cursor: null, + limit: 20, + }); + const archived = () => + listAccessibleProjects(ctx, { + userId: viewer, + archived: true, + cursor: null, + limit: 20, + }); + + expect((await active()).page.map((p) => p.name).sort()).toEqual([ + 'p', + 'shared', + ]); + expect((await archived()).page).toEqual([]); + + await ctx.orm + .update(projectsTable) + .set({ archived: true }) + .where(eq(projectsTable.id, project.id)); + await syncProjectArchived(ctx, { projectId: project.id, archived: true }); + + expect((await active()).page.map((p) => p.name)).toEqual(['shared']); + expect((await archived()).page.map((p) => p.name)).toEqual(['p']); + }); +}); + +/** + * Archiving a shared project has to move it for the members too, not just the + * owner -- the mirrored flag is part of the index range every viewer walks. + */ +test('archiving a shared project moves it for members as well', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const member = await makeUser(ctx, 'member'); + + const project = await makeProject(ctx, { name: 'shared', ownerId: owner }); + await addMember(ctx, { project, userId: member }); + + await ctx.orm + .update(projectsTable) + .set({ archived: true }) + .where(eq(projectsTable.id, project.id)); + await syncProjectArchived(ctx, { projectId: project.id, archived: true }); + + for (const userId of [owner, member]) { + const active = await listAccessibleProjects(ctx, { + userId, + archived: false, + cursor: null, + limit: 20, + }); + const archived = await listAccessibleProjects(ctx, { + userId, + archived: true, + cursor: null, + limit: 20, + }); + expect(active.page).toEqual([]); + expect(archived.page.map((p) => p.name)).toEqual(['shared']); + } + }); +}); + +test('revoking access hides the project from that user only', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const member = await makeUser(ctx, 'member'); + + const project = await makeProject(ctx, { name: 'shared', ownerId: owner }); + await addMember(ctx, { project, userId: member }); + + // Exactly what `projects.removeMember` does: drop the membership, then + // revoke. Revoke re-derives entitlement, so the order matters. + const membership = await ctx.orm.query.projectMembers.findFirst({ + where: { projectId: project.id, userId: member }, + }); + await ctx.orm + .delete(projectMembersTable) + .where(eq(projectMembersTable.id, membership.id)); + await revokeProjectAccess(ctx, { projectId: project.id, userId: member }); + + const forMember = await listAccessibleProjects(ctx, { + userId: member, + archived: false, + cursor: null, + limit: 20, + }); + const forOwner = await listAccessibleProjects(ctx, { + userId: owner, + archived: false, + cursor: null, + limit: 20, + }); + + expect(forMember.page).toEqual([]); + expect(forOwner.page.map((p) => p.name)).toEqual(['shared']); + }); +}); + +/** + * Revoke re-derives entitlement rather than deleting blindly, so a caller that + * revokes while the user is still the owner -- or still holds a membership -- + * cannot strip a project the user can legitimately see. + */ +test('revoking does not strip access the user is still entitled to', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const member = await makeUser(ctx, 'member'); + + const project = await makeProject(ctx, { name: 'shared', ownerId: owner }); + await addMember(ctx, { project, userId: member }); + + // The membership row is still there, and the owner still owns it. + await revokeProjectAccess(ctx, { projectId: project.id, userId: member }); + await revokeProjectAccess(ctx, { projectId: project.id, userId: owner }); + + for (const userId of [owner, member]) { + const results = await listAccessibleProjects(ctx, { + userId, + archived: false, + cursor: null, + limit: 20, + }); + expect(results.page.map((p) => p.name)).toEqual(['shared']); + } + + const rows = await ctx.orm.query.projectAccess.findMany({ + where: { projectId: project.id }, + limit: 100, + }); + expect(rows).toHaveLength(2); + expect(rows.find((row: AccessRow) => row.userId === owner)?.isOwner).toBe( + true + ); + expect(rows.find((row: AccessRow) => row.userId === member)?.isOwner).toBe( + false + ); + }); +}); + +/** + * `projects.transfer` moves ownership while keeping both users on the project. + * Re-granting an existing row must update it, never duplicate it. + */ +test('transferring ownership keeps one access row per user', async () => { + await withCountedOrmCtx(async (ctx) => { + const oldOwner = await makeUser(ctx, 'old'); + const newOwner = await makeUser(ctx, 'new'); + + const project = await makeProject(ctx, { name: 'p', ownerId: oldOwner }); + await addMember(ctx, { project, userId: newOwner }); + + // `projects.transfer` hands the post-transfer project to both grants. + const transferred = { ...project, ownerId: newOwner }; + await grantProjectAccess(ctx, { project: transferred, userId: newOwner }); + await grantProjectAccess(ctx, { project: transferred, userId: oldOwner }); + + const rows = await ctx.orm.query.projectAccess.findMany({ + where: { projectId: project.id }, + limit: 100, + }); + + expect(rows).toHaveLength(2); + expect( + rows.find((row: AccessRow) => row.userId === newOwner)?.isOwner + ).toBe(true); + expect( + rows.find((row: AccessRow) => row.userId === oldOwner)?.isOwner + ).toBe(false); + + for (const userId of [oldOwner, newOwner]) { + const results = await listAccessibleProjects(ctx, { + userId, + archived: false, + cursor: null, + limit: 20, + }); + expect(results.page.map((p) => p.name)).toEqual(['p']); + } + }); +}); + +/** + * Access rows are removed by foreign-key cascade, not by a trigger. That matters + * because async cascade continuation batches build their ORM from the raw + * `ctx.db` and drop triggers past the first batch, while cascades still run. + */ +test('deleting a project cascades its access rows away', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const member = await makeUser(ctx, 'member'); + + const project = await makeProject(ctx, { name: 'doomed', ownerId: owner }); + await addMember(ctx, { project, userId: member }); + + expect( + await ctx.orm.query.projectAccess.findMany({ + where: { projectId: project.id }, + limit: 100, + }) + ).toHaveLength(2); + + await ctx.orm.delete(projectsTable).where(eq(projectsTable.id, project.id)); + + expect( + await ctx.orm.query.projectAccess.findMany({ + where: { projectId: project.id }, + limit: 100, + }) + ).toEqual([]); + + for (const userId of [owner, member]) { + const results = await listAccessibleProjects(ctx, { + userId, + archived: false, + cursor: null, + limit: 20, + }); + expect(results.page).toEqual([]); + } + }); +}); + +/** + * The app-shell nav asks this on every route, so it must stay a single indexed + * read. Before `projectAccess`, the owner leg tied `index('archived')` and + * `index('ownerId')` at the same planner score, the earlier declaration won, and + * the loser became a post-filter over the whole non-archived partition. + */ +test('existence check reads nothing proportional to the table', async () => { + await withCountedOrmCtx(async (ctx, reads) => { + const other = await makeUser(ctx, 'other'); + const viewer = await makeUser(ctx, 'viewer'); + await seedNoise(ctx, other); + + const before = reads.scanned; + await expect(hasAnyProject(ctx, viewer)).resolves.toBe(false); + expect(reads.scanned - before).toBeLessThanOrEqual(2); + }); +}); + +test('existence check sees a membership-only project', async () => { + await withCountedOrmCtx(async (ctx, reads) => { + const other = await makeUser(ctx, 'other'); + const viewer = await makeUser(ctx, 'viewer'); + await seedNoise(ctx, other); + + const shared = await makeProject(ctx, { name: 'shared', ownerId: other }); + await addMember(ctx, { project: shared, userId: viewer }); + + const before = reads.scanned; + await expect(hasAnyProject(ctx, viewer)).resolves.toBe(true); + expect(reads.scanned - before).toBeLessThanOrEqual(2); + }); +}); + +/** + * A deployment seeded before `projectAccess` existed has no access rows. The + * backfill has to reconstruct exactly what the write path would have produced. + */ +test('backfill reconstructs the same access rows the write path maintains', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const member = await makeUser(ctx, 'member'); + + const project = await makeProject(ctx, { name: 'p', ownerId: owner }); + await addMember(ctx, { project, userId: member }); + const archivedProject = await makeProject(ctx, { + name: 'old', + ownerId: owner, + archived: true, + }); + + const snapshot = async () => + (await ctx.orm.query.projectAccess.findMany({ limit: 100 })) + .map((row: AccessRow) => + [ + row.userId, + row.projectId, + row.projectCreatedAt, + row.archived, + row.isOwner, + ].join('|') + ) + .sort(); + + const maintained = await snapshot(); + expect(maintained).toHaveLength(3); + + // Wipe the derived table, as a pre-`projectAccess` deployment would have + // it. A raw delete, not `revokeProjectAccess`, which would re-derive the + // rows straight back. + for (const row of await ctx.orm.query.projectAccess.findMany({ + limit: 100, + })) { + await ctx.orm + .delete(projectAccessTable) + .where(eq(projectAccessTable.id, row.id)); + } + expect(await snapshot()).toEqual([]); + + await backfillProjectAccess(ctx); + + expect(await snapshot()).toEqual(maintained); + void archivedProject; + }); +}); diff --git a/convex/orm/example-project-existence-reads.test.ts b/convex/orm/example-project-existence-reads.test.ts deleted file mode 100644 index bab4131f7..000000000 --- a/convex/orm/example-project-existence-reads.test.ts +++ /dev/null @@ -1,156 +0,0 @@ -import { expect, test } from 'vitest'; -import { hasAnyProject } from '../../example/convex/functions/_helpers/project_existence'; -import schema, { - projectMembersTable, - projectsTable, - userTable, -} from '../../example/convex/functions/schema'; -import { countDocumentReads, withOrmCtx } from '../setup.testing'; - -const EXAMPLE_ENV_DEFAULTS = { - ADMIN: 'admin@example.com', - BETTER_AUTH_SECRET: 'test-secret', - GITHUB_CLIENT_ID: 'github-client-id', - GITHUB_CLIENT_SECRET: 'github-client-secret', - GOOGLE_CLIENT_ID: 'google-client-id', - GOOGLE_CLIENT_SECRET: 'google-client-secret', -} as const; - -const withExampleEnv = async (run: () => Promise) => { - const original = Object.fromEntries( - Object.keys(EXAMPLE_ENV_DEFAULTS).map((key) => [key, process.env[key]]) - ); - - for (const [key, value] of Object.entries(EXAMPLE_ENV_DEFAULTS)) { - process.env[key] = value; - } - - try { - await run(); - } finally { - for (const [key, value] of Object.entries(original)) { - if (typeof value === 'string') { - process.env[key] = value; - } else { - delete process.env[key]; - } - } - } -}; - -const OTHER_USER_PROJECTS = 40; - -/** - * The app shell nav asks "does this user have at least one project" on every - * route. Answering that through the paginated `projects.list` read model - * attaches no index -- `orderBy({ createdAt })` normalizes to `_creationTime` -- - * and post-filters owner-or-member in JS, so a user with zero projects drains - * the whole table and every scanned row joins the subscription's read set. - * - * `projects.hasAny` answers the same non-archived owner-or-member question off - * `projects.ownerId` and `projectMembers.userId`. These tests call its exact - * source owner and pin that both legs stay index-backed: the read count must not - * track table size. - */ -test('project existence check does not scan projects owned by other users', async () => { - await withExampleEnv(async () => { - await withOrmCtx(schema, schema, async (ctx) => { - const [owner] = await ctx.orm - .insert(userTable) - .values({ - name: 'Owner', - email: 'existence-owner@test.dev', - emailVerified: true, - createdAt: new Date(), - updatedAt: new Date(), - }) - .returning({ id: userTable.id }); - - const [viewer] = await ctx.orm - .insert(userTable) - .values({ - name: 'Viewer', - email: 'existence-viewer@test.dev', - emailVerified: true, - createdAt: new Date(), - updatedAt: new Date(), - }) - .returning({ id: userTable.id }); - - for (let index = 0; index < OTHER_USER_PROJECTS; index += 1) { - await ctx.orm.insert(projectsTable).values({ - name: `other-${index}`, - isPublic: false, - archived: false, - ownerId: owner.id, - }); - } - - const reads = countDocumentReads(ctx); - - await expect(hasAnyProject(ctx, viewer.id)).resolves.toBe(false); - // Both legs hit an index range that is empty for this user. The - // unindexed read model reported OTHER_USER_PROJECTS reads here. - expect(reads.scanned).toBeLessThanOrEqual(2); - }); - }); -}); - -test('project existence check reads only the viewer own and member rows', async () => { - await withExampleEnv(async () => { - await withOrmCtx(schema, schema, async (ctx) => { - const [owner] = await ctx.orm - .insert(userTable) - .values({ - name: 'Owner', - email: 'existence-owner-2@test.dev', - emailVerified: true, - createdAt: new Date(), - updatedAt: new Date(), - }) - .returning({ id: userTable.id }); - - const [viewer] = await ctx.orm - .insert(userTable) - .values({ - name: 'Viewer', - email: 'existence-viewer-2@test.dev', - emailVerified: true, - createdAt: new Date(), - updatedAt: new Date(), - }) - .returning({ id: userTable.id }); - - for (let index = 0; index < OTHER_USER_PROJECTS; index += 1) { - await ctx.orm.insert(projectsTable).values({ - name: `other-${index}`, - isPublic: false, - archived: false, - ownerId: owner.id, - }); - } - - const [shared] = await ctx.orm - .insert(projectsTable) - .values({ - name: 'shared', - isPublic: false, - archived: false, - ownerId: owner.id, - }) - .returning({ id: projectsTable.id }); - - await ctx.orm.insert(projectMembersTable).values({ - projectId: shared.id, - userId: viewer.id, - }); - - const reads = countDocumentReads(ctx); - - await expect(hasAnyProject(ctx, viewer.id)).resolves.toBe(true); - // One membership and one related project row. Nothing proportional to - // OTHER_USER_PROJECTS. - expect(reads.scanned).toBeLessThanOrEqual(3); - }); - }); -}); diff --git a/convex/orm/example-tag-merge-reads.test.ts b/convex/orm/example-tag-merge-reads.test.ts index c30f13497..f88260e86 100644 --- a/convex/orm/example-tag-merge-reads.test.ts +++ b/convex/orm/example-tag-merge-reads.test.ts @@ -7,7 +7,31 @@ import schema, { todoTagsTable, userTable, } from '../../example/convex/functions/schema'; -import { countDocumentReads, withOrmCtx } from '../setup.testing'; +import { convexTest, countDocumentReads, withOrm } from '../setup.testing'; + +/** + * `countDocumentReads` swaps `db.query`/`db.get`, and the ORM binds both when it + * is built, so the counter has to be installed *before* `withOrm`. Installed on + * a context that already carries an ORM it counts only reads issued directly + * through `ctx.db` -- a constant zero for an ORM-only test, which makes every + * bound below pass vacuously. + */ +const withCountedOrmCtx = async ( + run: ( + ctx: { + db: Parameters[0]['db']; + orm: ReturnType>['orm']; + }, + reads: ReturnType + ) => Promise +): Promise => { + const t = convexTest(schema); + await t.run(async (baseCtx) => { + const reads = countDocumentReads(baseCtx); + const ctx = withOrm(baseCtx, schema); + await run(ctx, reads); + }); +}; const EXAMPLE_ENV_DEFAULTS = { ADMIN: 'admin@example.com', @@ -90,7 +114,7 @@ const seedTodo = async (ctx: ExampleCtx, userId: string, title: string) => { */ test('merge cost does not track the target tag size', async () => { await withExampleEnv(async () => { - await withOrmCtx(schema, schema, async (ctx) => { + await withCountedOrmCtx(async (ctx, reads) => { const userId = await seedUser(ctx, 'merge-reads@test.dev'); const sourceTagId = await seedTag(ctx, userId, 'Work'); const targetTagId = await seedTag(ctx, userId, 'work'); @@ -109,13 +133,13 @@ test('merge cost does not track the target tag size', async () => { .insert(todoTagsTable) .values({ todoId: sourceTodoId, tagId: sourceTagId }); - const reads = countDocumentReads(ctx); + const readsBefore = reads.scanned; await mergeTags(ctx, userId, { sourceTagId, targetTagId, }); - const mergeReads = reads.scanned; + const mergeReads = reads.scanned - readsBefore; const [sourceTag, targetJoins] = await Promise.all([ ctx.orm.query.tags.findFirst({ where: { id: sourceTagId } }), @@ -128,19 +152,27 @@ test('merge cost does not track the target tag size', async () => { expect(sourceTag).toBeNull(); expect(targetJoins).toHaveLength(TARGET_TAG_TODOS + 1); - expect(targetJoins.some((join) => join.todoId === sourceTodoId)).toBe( - true - ); + expect( + targetJoins.some( + (join: { todoId: string }) => join.todoId === sourceTodoId + ) + ).toBe(true); // The merge reads its two tags, one source join, one indexed target - // probe, and aggregate bookkeeping. It never materializes 40 target rows. - expect(mergeReads).toBeLessThanOrEqual(12); + // probe, and aggregate bookkeeping. It never materializes the target rows. + // + // 16 is measured, and it is the same at TARGET_TAG_TODOS 40 and 120, which + // is what this test exists to pin. The previous bound of 12 was never + // exercised: the counter was installed after the ORM was built, so it + // reported a constant zero and the assertion passed against nothing. + expect(mergeReads).toBeLessThanOrEqual(16); + expect(mergeReads).toBeLessThan(TARGET_TAG_TODOS); }); }); }); test('merge probe finds a target row the source todo already carries', async () => { await withExampleEnv(async () => { - await withOrmCtx(schema, schema, async (ctx) => { + await withCountedOrmCtx(async (ctx, reads) => { const userId = await seedUser(ctx, 'merge-reads-2@test.dev'); const sourceTagId = await seedTag(ctx, userId, 'Work'); const targetTagId = await seedTag(ctx, userId, 'work'); @@ -161,7 +193,7 @@ test('merge probe finds a target row the source todo already carries', async () .insert(todoTagsTable) .values({ todoId: sharedTodoId, tagId: sourceTagId }); - const reads = countDocumentReads(ctx); + const readsBefore = reads.scanned; await mergeTags(ctx, userId, { sourceTagId, @@ -181,7 +213,7 @@ test('merge probe finds a target row the source todo already carries', async () expect(targetJoins).toHaveLength(1); // The probe is index-backed: the merge must not scan the target's other // rows or insert a duplicate for the shared todo. - expect(reads.scanned).toBeLessThanOrEqual(20); + expect(reads.scanned - readsBefore).toBeLessThanOrEqual(20); }); }); }); diff --git a/convex/setup.testing.ts b/convex/setup.testing.ts index 452a24f69..4b6e2f1a6 100644 --- a/convex/setup.testing.ts +++ b/convex/setup.testing.ts @@ -118,6 +118,47 @@ export function convexTest>( return wrapConvexTestDateReturns(baseConvexTest(schema, convexModules)); } +/** + * Placeholder values for the environment `example`'s `getEnv()` demands. + * + * Every suite that drives example source needs these present, and none of them + * cares what they contain. They live here so the same block is not copied into + * each test file. + */ +const EXAMPLE_ENV_DEFAULTS = { + ADMIN: 'admin@example.com', + BETTER_AUTH_SECRET: 'test-secret', + GITHUB_CLIENT_ID: 'github-client-id', + GITHUB_CLIENT_SECRET: 'test-secret', + GOOGLE_CLIENT_ID: 'google-client-id', + GOOGLE_CLIENT_SECRET: 'test-secret', +} as const; + +/** Run `fn` with the example app's required env vars set, then restore them. */ +export const withExampleEnv = async ( + fn: () => Promise +): Promise => { + const original = Object.fromEntries( + Object.keys(EXAMPLE_ENV_DEFAULTS).map((key) => [key, process.env[key]]) + ); + + for (const [key, value] of Object.entries(EXAMPLE_ENV_DEFAULTS)) { + process.env[key] = value; + } + + try { + await fn(); + } finally { + for (const [key, value] of Object.entries(original)) { + if (typeof value === 'string') { + process.env[key] = value; + } else { + delete process.env[key]; + } + } + } +}; + export type DocumentReadCounts = { /** * Documents handed back to the caller. @@ -518,8 +559,16 @@ export function countDocumentReads(ctx: { }); (ctx.db as any).query = (table: unknown) => wrap(baseQuery(table as never)); - (ctx.db as any).get = async (id: unknown) => { - const row = await baseGet(id as never); + // Forward every argument. `db.get` has a 1-arg and a 2-arg overload, and the + // lifecycle writer only ever calls the 2-arg one -- `innerDb.get(table, id)` + // at lifecycle.ts:383, 397, 472, 554, 627. A 1-arg forwarder drops the id, so + // `oldDoc`/`newDoc` come back null and the writer skips every trigger without + // erroring: installing the counter silently disabled all triggers, and any + // read bound measured over a trigger-maintained table passed against zero. + (ctx.db as any).get = async (...args: unknown[]) => { + const row = await ( + baseGet as (...forwarded: unknown[]) => Promise + )(...args); if (row) { reads.documents += 1; reads.scanned += 1; diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md new file mode 100644 index 000000000..54659fc5a --- /dev/null +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -0,0 +1,549 @@ +# 443 projects.list read bounds + +Objective: +Make `example` `projects.list` reads independent of the `projects` table size and narrow its reactive read set, preserving owner-or-member visibility, archived-only toggle, createdAt-desc order, and cursor pagination. + +Goal plan: +docs/plans/2026-09-05-443-projects-list-read-bounds.md + +Template: +docs/plans/templates/task.md + +Primary template: +docs/plans/templates/task.md + +Applied packs: +- none + +Task source: +- type: GitHub issue +- id / link: #443 โ€” https://github.com/udecode/kitcn/issues/443 +- title: Example: `projects.list` scans the whole table to authorize owner-or-member, and every scanned row joins the read set +- acceptance criteria: + 1. `projects.list` reads do not track `projects` table size. + 2. The reactive read set excludes projects the viewer cannot see. + 3. Owner-or-member visibility, archived-only toggle, `createdAt desc` order, + cursor pagination, and `isDone` are unchanged. + 4. A `countDocumentReads` regression test pins the bound. +- caveats: issue's "empty page after full scan" correctness claim is NOT a bug + (see Pre-solution issue challenge). Issue's preferred `projectAccess` denormalized + table has a confirmed async-cascade trigger-drop hazard. +- likely files: `example/convex/functions/projects.ts`, + `example/convex/functions/schema.ts`, + `example/convex/functions/_helpers/`, `convex/orm/example-*-reads.test.ts` +- browser surface: `/projects` (one consumer, `example/src/app/projects/page.tsx:45-47`) +- root-cause layer: example app read model (not ORM) + +Timed checkpoint: +- requested duration: pending +- semantics: pending +- initial confidence score: pending +- improvement loop: pending +- final score / loop closure: pending + +Completion threshold: +- `projects.list` read cost is independent of `projects` table size, pinned by a + `countDocumentReads` regression test that fails if the counter is dead. +- Task closure is legal only when the source-of-truth acceptance criteria are + satisfied or explicitly narrowed, required verification evidence is recorded, + code-review and release-artifact gates are closed when applicable, verified + code changes are committed and PR'd unless explicitly declined or blocked, + task-style PR body sync is complete or marked N/A with reason, + GitHub issue/PR sync is complete or marked N/A with reason, and + `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/2026-09-05-443-projects-list-read-bounds.md` passes. + +Verification surface: +- `convex/orm/example-project-access-reads.test.ts`, `bun typecheck`, + `bun run test`, `bun lint:fix`, `bun check`, autoreview. + +Constraints: +- Preserve existing user-facing behavior outside the task scope. +- Prefer the durable ownership boundary over caller-by-caller patches. +- When a GitHub PR is in scope, this plan owns exactly one PR. A coordinating + batch plan must link a separate task plan for every PR an agent processes. +- Verified code changes must be committed and PR'd because the task skill + requires that path unless the user explicitly says not to, the work has no + local patch, or a real blocker is recorded. +- The absence of a separate "open a PR" sentence from the user is not a valid + N/A reason for verified code-changing task work. +- A PR created by this task must use the PR #270 emoji task-style PR body + contract below, not a generic summary/body from a git helper skill. +- A task-run PR body must include + `๐Ÿงญ Task plan: docs/plans/.md`; the plan must exist at the PR head and + identify the exact PR before autoclosure. +- Do not add broad ceremony when the task is trivial or docs-only. + +Boundaries: +- Source of truth: GitHub issue #443. +- Allowed edit scope: `example/convex/**`, `convex/orm/**`, `convex/setup.testing.ts`. +- Browser surface: `/projects`. Not exercised โ€” this checkout has no Convex + deployment (only `.env.example` exists anywhere in the repo or its source + checkout), so the app cannot be started. The change has no rendered-output + intent; behavior is pinned by tests instead. +- GitHub issue sync: not posted. Posting is outward-facing and the user's only + standing instruction was "no PR"; offered in the handoff instead. +- Non-goals: the ORM union-source id-IN lowering, the `.filter()` raw-row type + lie, and the async-cascade trigger drop. All recorded as follow-ups. + +Output budget strategy: +- Exploration ran as two bounded background workflows whose reports were read + from the journal rather than streamed. Test output filtered with `grep`/`tail`. + +Blocked condition: +- None reached. Browser proof and `convex codegen` for `_generated/dataModel.d.ts` + are unavailable because no Convex deployment is configured; both are recorded + as caveats rather than blockers. + +Task state: +- task_type: bug +- task_complexity: non-trivial +- current_phase: closeout +- current_phase_status: done +- next_phase: final response +- goal_status: complete + +Current verdict: +- verdict: partially valid, fixed +- confidence: 95-100% on the read-bound and correctness claims; browser proof + waived (no Convex deployment exists in this checkout) +- next owner: task +- reason: the scan is real and fixed; the claimed correctness bug is not a bug; + two larger defects the issue missed are fixed alongside it + +Implementation readiness: +- verdict: ready +- exact owner: `example/convex/functions/_helpers/project_access.ts` +- contradiction status: resolved โ€” the issue's "empty page" claim contradicts + the UI copy and the stream source; the UI and source agree with each other, + so the claim is invalid and no code changed for it. +- source-listed cases complete: yes, see the case matrix + +Pre-solution issue challenge: +- reporter claim: + (a) `projects.list` walks `projects` unindexed and stops only after `limit` + authorized rows match, so a viewer with few/no projects drains the table + and every scanned row joins the reactive read set. + (b) "correctness bug riding along": an `includeArchived: true` user with no + archived projects gets an empty page after a full scan. +- suggested diagnosis or fix: add a denormalized `projectAccess` table indexed + `(userId, archived, projectCreatedAt)` maintained by schema triggers; the + alternative of extending `FindManyUnionSource` with a table selector is + called a larger ORM change for the same outcome. +- repro ladder: + - tests / source-level repro: DONE. Scratch vitest against the real example + schema, viewer owning 0 of 40 projects: `scanned: 40`, `page: 0`, + `isDone: true`. Read count tracks table size exactly. + - repo-owned automated browser or integration proof: N/A โ€” the defect is a + read-count/read-set property, invisible in rendered output. + - Browser plugin: pending (route `/projects`; behavior must stay identical) + - screenshot / visual proof: N/A โ€” no rendered-output change intended. +- reproduction verdict: valid for claim (a); claim (b) NOT reproduced as a bug. +- validity verdict: **partially valid**. + - (a) CONFIRMED. Real, and worse than described: it costs O(table) per page + for every authenticated caller at both `includeArchived` values. + - (b) INVALID as a correctness bug. `includeArchived` means archived-ONLY + (`projects.ts:174-176` `return project.archived`), and the UI agrees โ€” + `example/src/app/projects/page.tsx:187-192` labels it "Show archived only" + and `:262-268` renders "No archived projects". An empty page for a user + with no archived projects is correct. `isDone` is also correct: rejected + rows push to `indexKeys` but not `page`, and the loop breaks on + `page.length >= maxRows` (`packages/kitcn/src/orm/stream.ts:399-424`), + so stream exhaustion yields `isDone: true`. The archived-with-none case is + the worst instance of (a), not a distinct defect. +- best long-term fix boundary: the example app read model. Not the ORM: a + compound index already makes the owned leg fully index-served + (`packages/kitcn/src/orm/index-utils.ts:194-205`, measured constant at 60 and + 300 rows), and id-IN already lowers to point lookups on non-union paths + (`query.ts:2910-2985`, reached at `:6186`). +- harsh honest feedback on the issue's proposed path: + - The suggested `projectAccess` table is NOT free of ORM risk. Async/scheduled + cascade continuation workers build their ORM from the raw `ctx.db` + (`packages/kitcn/src/orm/scheduled-mutation-batch.ts:118`, + `scheduled-delete.ts:55`) and never wrap the lifecycle, so triggers are + DROPPED past the first batch. Measured with `mutationBatchSize: 2` and 6 + children: all 6 rows deleted, only 2 fired triggers. The example app runs in + inferred async mode. A trigger-maintained derived table can therefore drift. + - The issue's framing "it is no longer gated on ORM work" is right about the + conclusion but wrong about the reason: it is not gated on ORM work because + the ORM's existing index-order pushdown already solves the owned leg. +- hard-stop decision: proceed on (a); do not implement anything for (b), and + say so in the issue sync. + +Completion rule: +- Do not call `update_goal(status: complete)` while any required checklist item + remains unchecked. If an item does not apply, check it and add `N/A: `. +- Do not call `update_goal(status: complete)` until every completion threshold + above is satisfied, final handoff evidence is recorded, and + `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/2026-09-05-443-projects-list-read-bounds.md` passes. +- Do not create hook state for this goal. This file plus the active goal are the + durable state. + +Start Gates: +| Gate | Applies | Evidence | +|------|---------|----------| +| Timed checkpoint parsed | no | N/A: no duration requested | +| Walkthrough baseline for possible UI change | no | N/A: server read model + tests only, no rendered-output change | +| Skill analysis before edits | yes | task + autogoal + autoreview loaded; no others earned their keep | +| Active goal checked or created | yes | this plan | +| Source of truth read before edits | yes | gh issue view 443 + projects.ts + schema.ts read first | +| Exact per-PR task ownership | no | N/A: user declined PRs | +| GitHub comments and attachments read | yes | issue has zero comments (gh --json comments returned []) | +| Video transcript evidence required | no | N/A: no video in source | +| Pre-solution issue challenge required | yes | recorded above; verdict partially valid | +| Reproduction verdict before implementation | yes | scanned: 40 / page: 0 captured before any edit | +| Repro escalation ladder selected | yes | source-level vitest repro; browser N/A, no deployment | +| Suggested fix reviewed against durable boundary | yes | issue's trigger-maintained variant rejected for the async-cascade drop; explicit helpers chosen | +| `docs/solutions` checked for non-trivial existing-code work | no | N/A: directory does not exist in this repo | +| TDD decision before behavior change or bug fix | yes | repro first, then helper, then regression suite | +| Branch decision for code-changing task | yes | already on dedicated branch issue-443-task | +| Release artifact decision | no | N/A: example + test-infra only; .changeset/config.json ignores example, nothing under packages/ changed | +| Browser tool decision for browser surface | yes | waived: no Convex deployment exists, so the app cannot start | +| Commit / PR expectation decision | no | N/A: explicit user decline โ€” "Do not create PR under any circumstances, unless user prompts to" | +| Task-style PR body decision | no | N/A: no PR | +| Task-plan PR body evidence | no | N/A: no PR | +| GitHub issue sync expectation decision | no | not posted; outward-facing, offered to the user instead | +| Output budget strategy recorded | yes | recorded above | + +Work Checklist: +- [x] If a duration was requested, it is recorded as minimum active work unless + explicitly marked hard stop; when no better metric exists, initial and + final confidence scores are recorded. +- [x] Objective includes outcome, completion threshold, verification surface, + constraints, boundaries, and blocked condition. +- [x] Task source classified with source type, id/link, title, task type, + acceptance criteria, caveats, likely files/routes/packages, browser + surface, and root-cause layer. +- [x] Every GitHub PR in scope has its own task plan. This plan owns one exact + PR, owns a not-yet-created PR slice, or records N/A because no PR is in + scope; a batch plan is not used as a substitute. +- [x] Required video or screen-recording evidence is cached/read as normalized + `` XML, or marked N/A with reason. +- [x] For public GitHub bug reports, behavior claims, technical diagnoses, or + suggested fixes, reporter claims are challenged before implementation + with a recorded verdict: `valid`, `not reproduced`, `invalid`, + `wont-fix`, `partially valid`, or `platform limitation`. Feature, docs, + support, or cleanup requests with no bug claim may mark reproduction + `N/A` with reason. +- [x] Repro escalation ladder followed for bug/behavior claims: focused + test/source-level repro first when applicable; existing repo-owned + automated browser or integration proof next when available and useful as + executable coverage; the repo-approved Browser tool next when tests or + automation cannot reproduce or cannot model the surface honestly; + screenshot or explicit visual-proof waiver when visual/native state + matters. +- [x] Hard-stop rule followed for bug/behavior claims: no code when the issue + is not reproduced, invalid, or won't-fix; partial validity pivots to the + best long-term fix and records what was wrong or incomplete in the + issue's proposed path. +- [x] Nearby repo instructions and implementation patterns read before edits. +- [x] Source-listed case matrix is complete and every contradiction has an + owner, harness, and verdict before mutation. +- [x] Readiness is classified `ready`, `repair-source`, `major`, `blocked`, or + `invalid` with evidence. +- [x] Implementation fixes the right ownership boundary, or the narrower choice + is recorded with reason. +- [x] Release artifact requirement recorded: active changeset, new changeset, or + N/A with reason. +- [x] Final handoff shape decided: bug/feature/testing/batch/review/GitHub + requirements, PR body sync, and issue sync when applicable. +- [x] Commit/PR handling recorded for code-changing work: commit and PR + completed, no local patch, user explicitly declined, or blocker recorded. + "User did not separately ask for a PR" is not a valid blocker. +- [x] PR body shape recorded: PR #270 emoji task-style body used, N/A reason + recorded, or blocker recorded. +- [x] PR task evidence recorded: body includes `๐Ÿงญ Task plan: ...`, the plan + exists at the PR head, and it identifies the exact PR before autoclosure. +- [x] Branch handling recorded for code-changing work: dedicated branch used, + new branch needed, or N/A with reason. +- [x] Local-env-rot retry policy recorded for any surprising repo-wide failure: + reinstall/rerun evidence or N/A with reason. +- [x] Workspace authority recorded: every proof command names the cwd/tool that + owns the changed behavior. +- [x] Output budget discipline recorded and followed: broad searches are + scoped, capped, counted, or artifacted instead of streamed into goal + context. +- [x] High-risk note recorded for public API, runtime, package-boundary, + browser behavior, agent-action, or command-contract changes, or marked + N/A with reason. +- [x] Review/autoreview target selected from actual diff state for non-trivial + implementation work, or marked N/A with reason. +- [x] Agent-native review decision recorded for `.agents/**`, `.claude/**`, + `.codex/**`, skills, hooks, commands, prompts, or user-action tooling. + +Completion Gates: +| Gate | Applies | Required action | Evidence | +|------|---------|-----------------|----------| +| Named verification threshold | yes | Ran | `bunx vitest run convex/orm/example-project-access-reads.test.ts` โ€” 13 pass; cost 4 reads at 10 and at 400 noise rows | +| Exact per-PR task ownership | no | N/A | user declined PRs | +| Pre-solution issue challenge verdict | yes | Recorded | partially valid; claim (b) invalid, no code written for it | +| Repro escalation ladder | yes | Recorded | source-level repro captured; browser N/A (no deployment) | +| Bug reproduced before fix | yes | Captured | scanned 40 / page 0, and member-only project invisible at page 0 of 31 | +| Targeted behavior verification | yes | Ran | 13-test access suite + 554 vitest ORM tests | +| TypeScript or typed config changed | yes | Ran | `bun typecheck` 5/5 successful | +| Package exports or file layout changed | no | N/A | nothing under packages/ changed | +| Package manifests, lockfile, or install graph changed | no | N/A | no manifest or lockfile change | +| Agent rules or skills changed | no | N/A | no .agents/.claude changes | +| Workspace authority proof | yes | Recorded | all commands run from the worktree root /Users/mikey/conductor/workspaces/kitcn/montgomery | +| Browser surface changed | no | Waived | no Convex deployment exists in this checkout or its source worktree, so the app cannot start; no rendered-output change intended | +| Browser final proof | no | Waived | same reason | +| UI walkthrough | no | N/A | no UI or rendered-output change | +| Scaffold or fixture output changed | no | N/A | `git status -- fixtures/ packages/kitcn/src/cli/registry/` is empty; the expo drift in `bun check` is an upstream patch bump | +| Package behavior or public API changed | no | N/A | example + test infra only; .changeset/config.json ignores example | +| Docs and kitcn skill sync changed | no | N/A | no www/** or skills content touched | +| Docs or content changed | no | N/A | plan doc only | +| High-risk mini gate | yes | Recorded | see High-risk note below | +| Agent-native review for agent/tooling changes | no | N/A | no agent-native surface touched | +| Local install corruption suspected | no | N/A | no corruption-shaped failure appeared | +| Commit created | no | N/A | explicit user decline of PRs; work left in the worktree for review | +| PR create or update | no | N/A | explicit user decline | +| Task-style PR body verified | no | N/A | no PR | +| PR task evidence verified | no | N/A | no PR | +| PR proof image hosting | no | N/A | no PR | +| GitHub issue sync-back | no | Not posted | outward-facing publish; user's standing instruction covered PRs only, so offered in the handoff instead | +| Final handoff contract | yes | Filled | see Final handoff contract | +| Final lint | yes | Ran | `bun lint:fix` clean; `biome check` reports no fixes needed | +| Output budget discipline | yes | Recorded | workflow reports read from journal, command output filtered | +| Timed checkpoint | no | N/A | no duration requested | +| Autoreview for non-trivial implementation changes | yes | Ran | `autoreview --mode local --engine claude` โ€” clean, no accepted/actionable findings (3 passes; findings from passes 1-2 fixed) | +| Goal plan complete | yes | Run `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/2026-09-05-443-projects-list-read-bounds.md` | passes | + +Phase / pass table: +| Phase | Status | Evidence | Next | +|-------|--------|----------|------| +| Intake and source read | done | issue + code read, repro captured | implementation | +| Implementation | done | schema table + helper + call sites + harness fix | verification | +| Verification | done | see Verification evidence | closeout | +| Commit / PR / GitHub sync | N/A | user explicitly declined PRs ("Do not create PR under any circumstances, unless user prompts to"); work left uncommitted in the worktree | final response | +| Closeout | done | autoreview clean, all gates run | final response | + +Findings: +- REPRO: viewer owning 0 of 40 projects โ†’ `scanned: 40`, `page: 0`. Confirmed. +- `createdAt` is a system alias for `_creationTime` + (`packages/kitcn/src/orm/builders/system-fields.ts:107-127`) and can never be + named in an index (`table.ts:787-795` throws). +- **A compound index whose fields are all pinned as equalities DOES serve + `orderBy createdAt desc` from Convex's implicit trailing `_creationTime` key** + (`packages/kitcn/src/orm/index-utils.ts:194-205`). Measured: constant 3 reads + at both 60 and 300 table rows, cursor-paginated and not. So + `index('ownerId_archived')` makes the owned leg O(page) with no ORM change. +- `where: { id: { in: [...] } }` lowers to per-id `db.get()` point lookups on + non-union paths (`query.ts:2910-2985`, reached at `:6186`) โ€” O(|ids|). +- Union sources cannot do that: `_buildUnionSourceStream` (`query.ts:3257-3295`) + hardcodes `.query(this.tableConfig.name)` and turns `where` into a JS + `filterWith`. Measured 118 reads at 60 rows, 598 at 300 rows (~2N). Union and + id-lookup are mutually exclusive branches (`query.ts:6138`). +- Triggers are dropped on async cascade continuation batches + (`scheduled-mutation-batch.ts:118`, `scheduled-delete.ts:55`). Measured. This + also means `aggregateIndex('by_project')` on `projectMembers` + (`example/convex/functions/schema.ts:429`) drifts on large cascading deletes โ€” + a latent bug worth its own issue. +- **The existing regression guards are vacuous.** `countDocumentReads` must be + installed before the ORM is built (`convex/setup.testing.ts:320-334`), but + `convex/orm/example-project-existence-reads.test.ts:89` and + `example-tag-merge-reads.test.ts` install it inside `withOrmCtx`. My probe: + same schema, 40 real ORM scans, counter reported `scanned: 0`. Their + `toBeLessThanOrEqual` assertions pass against a constant 0. +- **`hasAnyProject` is unbounded too, and its guard never saw it.** With the + counter correctly placed, `convex/orm/example-project-existence-reads.test.ts` + fails at 40 (bound 2) and 43 (bound 3) โ€” exactly `OTHER_USER_PROJECTS`. Cause: + `where: { ownerId, archived: false }` ties `index('archived')` and + `index('ownerId')` at planner score 76, and the stable sort keeps declaration + order, so `archived` wins and `ownerId` becomes a JS post-filter over the + whole non-archived partition. So PR #334 moved the app-shell nav off one + unbounded read and onto another. `index('ownerId_archived')` fixes it. +- ADJACENT (same function, unauth branch): `where: { isPublic: true, archived: false }` + has no compound index. The planner ties `index('isPublic')` and + `index('archived')` at 76 and picks `isPublic` by declaration order, so + `archived` is a JS post-filter and every archived public project is read and + subscribed to. +- One consumer: `example/src/app/projects/page.tsx:45-47`, route `/projects`, + "Load more" so cursor pagination is genuinely exercised. +- `.changeset/config.json:12` ignores `example` โ†’ example-only changes need no + changeset and no `packages/kitcn` build. + +Decisions and tradeoffs: +- **A second, unreported defect dominates the issue.** `.filter()` hands the + callback the RAW Convex row (`_creationTime`, `_id`, `archived`, `createdAt`, + `isPublic`, `name`, `ownerId` โ€” no `id`), because `query.ts:3433-3438` calls + `stage.filterWith(row)` without `_toPublicRow`. So + `memberProjectIds.has(project.id)` is `has(undefined)` and is ALWAYS false. + Verified myself: a viewer who is a genuine member of one non-archived project + gets `page: 0` after scanning all 31 rows. **Member-only projects have never + appeared in `projects.list`.** Fixing the read model necessarily restores + them, so "preserve exact current behavior" is unsatisfiable as written; the + restored visibility is the correct behavior and must be disclosed. +- **Chosen: a denormalized `projectAccess` read model, maintained by explicit + mutation helpers, NOT by schema triggers.** + - Why denormalized: it is the only shape that is ordering-correct by + construction. One index walk means a native Convex cursor, native `isDone`, + and no hand-rolled comparator. Measured 2 reads/row, flat at N=60 and N=400. + - Why NOT a JS merge of two index-served legs (the "minimal" alternative): + `_creationTime` carries sub-millisecond fractions + (`โ€ฆ482.002`, `โ€ฆ482.005`) that the hydrated `createdAt` Date truncates to ms. + Any cross-leg comparator over `createdAt.getTime()` therefore disagrees with + the index order of the legs it is merging, and it cannot be fixed from + `Select<'projects'>`, which only exposes the truncated Date. That design + also needs a hand-rolled composite cursor and a guessed refill budget in a + file that doubles as a docs source. + - Why NOT schema triggers: async/scheduled cascade continuation workers build + their ORM from the raw `ctx.db` and drop triggers past the first batch + (measured). The repo's own guidance covers this exact case โ€” + `packages/kitcn/skills/kitcn/SKILL.md:25`: "Prefer schema triggers for + cross-row invariants, but move invariant maintenance to explicit mutation + helpers if trigger execution is unstable." It is unstable here. Explicit + helpers also keep `convex/orm/example-schema-triggers.test.ts`'s + "registers triggers only on user/session tables" shape intact. + - Deletion stays FK-driven: `projectAccess.projectId` and `.userId` both + cascade, so no delete call site and no trigger is needed, and cascade + continuation batches still remove the rows. + - Why NOT the ORM change (union-source id-IN lowering): it is a real ORM + defect but it is not required to fix this issue, it drags a changeset + + package build + `www/**` and skills doc sync into an example-only bug, and + the example read model it enables is O(memberships) per page rather than + page-bounded. Recorded as a follow-up instead. +- Scope kept to one coherent slice: one canonical owner + (`_helpers/project_access.ts`) for "which projects can this user see", used by + `list`, `listForDropdown`, and `hasAny`, so the change ends with one + implementation of that question instead of three. +- Out of scope, recorded for follow-up issues: + - `listForDropdown`'s `{ id: { in }, archived }` full-table scan caused by + `_extractIdOnlyWhere` requiring `keys.length === 1` (`query.ts:864`). This + change removes the call site; the ORM defect remains. + - The `.filter()` raw-row type lie (`types.ts:110` promises the public shape, + `query.ts:3433-3436` delivers the raw one) โ€” root cause of the dead member + leg, touches every `.filter`/`.map`/`.distinct` caller. + - Async cascade continuation dropping triggers, which also drifts + `aggregateIndex` counts. + +Implementation notes: +- None yet. + +Review fixes: +- None yet. + +Error attempts: +| Error / failed attempt | Count | Next different move | Resolution | +|------------------------|-------|---------------------|------------| +| None yet | 0 | | | + +Verification evidence: +- Repro (before), `bunx vitest`, cwd repo root: viewer owning 0 of 40 projects โ†’ + `scanned: 40`, `page: 0`. Viewer who is a genuine member of 1 of 31 โ†’ + `page: 0`, filter-callback row keys + `[_creationTime,_id,archived,createdAt,isPublic,name,ownerId]` (no `id`). +- `convex/orm/example-project-access-reads.test.ts` โ€” 12 tests, all pass. + Measured cost `4` reads at 10 noise rows and `4` at 400: exactly + `2 x visible projects`, flat across a 40x table. Asserted as an equality so a + counter reporting zero fails instead of passing. +- `bunx vitest run convex/orm/` โ€” 553 passed, 14 skipped, 0 failed. +- `bun typecheck` โ€” 5/5 tasks successful. +- `bun run test` โ€” 1400 bun tests pass / 0 fail; 991 vitest pass, 14 skipped, + no type errors. +- `bun lint:fix` โ€” clean. +- `bun check` โ€” lint, typecheck, test, test:cli, test:concave all pass. Fails + only at `fixtures:check` on upstream expo drift (`~55.0.30` โ†’ `~55.0.31`). + `git status -- fixtures/ packages/kitcn/src/cli/registry/` is empty, so the + drift is not from this diff. +- Tag-merge guard corrected: honest cost is `16`, identical at + `TARGET_TAG_TODOS` 40 and 120, so the "cost does not track target size" + property holds; the old bound of `12` had never been exercised. + +Source-listed case matrix: +| Case | Source claim | Harness | Before | Expected after | Evidence | Status | +| --- | --- | --- | --- | --- | --- | --- | +| Unbounded list scan | `list` drains the table to authorize owner-or-member | `example-project-access-reads.test.ts` read bounds | 40 scanned for a viewer with 0 projects | O(page), flat in table size | 4 reads at 10 and at 400 noise rows | fixed | +| Read set | every scanned row joins the subscription | same | whole table | only the viewer's own access rows + their projects | index range is per-user | fixed | +| `includeArchived` empty page | claimed correctness bug | source + UI read | empty page | unchanged โ€” empty is correct | `projects.ts` returns archived-only; UI says "Show archived only" / "No archived projects" | invalid, no change | +| `isDone` | implied incorrect | `stream.ts:399-424` trace | correct | correct | rejected rows push to `indexKeys`, not `page` | invalid, no change | +| Member projects invisible | NOT in the issue | `example-project-access-reads.test.ts` | member-only project never listed | listed | dedicated test + repro | fixed | +| `hasAnyProject` unbounded | NOT in the issue | same suite | 40 / 43 reads | <= 2 | dedicated test | fixed | +| Vacuous read guards | NOT in the issue | harness arity fix | counter reported 0 | counter is live | `setup.testing.ts` forwards both args | fixed | +| Unauth branch post-filter | NOT in the issue | `index('isPublic_archived')` | `archived` post-filtered | exact index match | schema change | fixed | + +Final handoff contract: +- Commit line: none โ€” user explicitly declined PRs; changes left in the worktree +- PR line: N/A +- Issue line: not posted; offered to the user +- Confidence line: 95-100% on the read bound and the correctness fix +- Flow table: + - Reproduced: tests yes (scanned 40 / page 0; member-only invisible), browser N/A + - Verified: tests yes (13-test suite, 4 reads flat at 10 and 400), browser N/A +- Browser check: waived โ€” no Convex deployment exists in this checkout +- Outcome: `projects.list` reads are now O(page) and per-user; member-only + projects are visible for the first time; `hasAnyProject` is index-bounded; the + read-bound test harness no longer reports a constant zero +- Caveat: `_generated/dataModel.d.ts` not regenerated (no deployment); + `backfillProjectAccess` not wired to a migration; cursors change shape +- Design: + - Chosen boundary: `example/convex/functions/_helpers/project_access.ts` as the + single owner of "which projects can this user see" + - Why not quick patch: adding an index alone cannot answer owner-OR-member from + one ordered stream, and a JS merge of two legs orders by a ms-truncated + `createdAt` that disagrees with the index order of the legs it merges + - Why not broader change: the ORM union-source id-IN lowering is a real defect + but is not needed here, and would drag a changeset + package build + doc sync + into an example-only bug +- Verified: `bun typecheck` 5/5, `bun run test` 1400 bun + 992 vitest / 0 fail, + `bun lint:fix` clean, autoreview clean +- PR body verified: N/A โ€” no PR + +Task-style PR body contract: +- Preserve any existing `` block. If a changeset is + part of the diff and repo policy expects auto release, include that block. +- Use the accepted PR #270 visual format. The body starts with an emoji + issue/fix line, for example `๐Ÿ› Fixes #123` or `๐Ÿ› Fixes โž– N/A`, then + `๐Ÿงญ Task plan: docs/plans/.md`, then an emoji confidence line like + `๐ŸŸข 95-100% confidence`. +- Use this exact table header: `| Phase | ๐Ÿงช Tests | ๐ŸŒ Browser |`. +- Use `Reproduced` and `Verified` rows. Mark passing proof with `๐ŸŸข`, repro or + failing proof with `๐Ÿ”ด`, and non-applicable cells with `โž– N/A`. +- Use bold emoji section headings: `**โœ… Outcome**`, `**โš ๏ธ Caveat**`, + `**๐Ÿ—๏ธ Design**`, and `**๐Ÿงช Verified**`. +- Never include a line that links to the current PR itself. The current PR URL + belongs in the final response, not in its own description. +- Do not replace this with a generic `Summary` / `Verification` PR body, an + adaptive prose body from a git helper skill, plain `## Outcome` sections, or + an unrelated generated badge footer unless the caller or repo template + explicitly asks for it. +- Proof is `gh pr view --json body` output or a concise source-backed summary + of that output. + +Final handoff / sync: +- Commit: none (explicit user decline) +- PR: N/A +- Issue: not posted; offered +- Browser proof: waived, no deployment +- Caveats: see Open risks + +Timeline: +- 2026-09-05T06:07:24.857Z Task goal plan created. + +Reboot status: +| Question | Answer | +|----------|--------| +| Where am I? | Closeout complete; awaiting the user on commit/PR/issue sync | +| Where am I going? | Final response only | +| What is the goal? | Make `projects.list` reads independent of table size and narrow its read set | +| What have I learned? | See Findings โ€” three defects, only one of which the issue reported | +| What have I done? | See Timeline and Verification evidence | + +Open risks: +- `_generated/dataModel.d.ts` was NOT regenerated: `convex codegen` needs a + configured deployment and none exists in this checkout or its source + worktree. `kitcn codegen` did run and updated + `example/convex/functions/generated/procedure-names.gen.ts`. Anyone with a + deployment should run `bun --cwd example run codegen` before deploying. +- `backfillProjectAccess` is tested but not wired to a callable migration. The + migrations manifest is CLI-generated ("Do not edit manually") and + `kitcn migrate create` needs a deployment. An existing deployment shows empty + project lists until it is run; the example app is normally re-seeded. +- Pagination cursors change shape (they now page `projectAccess`, not + `projects`). A cursor issued before the deploy is not valid after it. +- Sort key is millisecond-resolution, so two projects created in the same + millisecond are separated by access-row creation order rather than by the + sub-millisecond `_creationTime`. Documented at both the schema and the helper. + +Hard closeout guard: +- A local-only final response for verified code-changing work is invalid unless + this plan records an explicit user decline, no local patch, analytical/ + blocked/inconclusive outcome, or a real commit/PR blocker. diff --git a/example/convex/functions/_helpers/project_access.ts b/example/convex/functions/_helpers/project_access.ts new file mode 100644 index 000000000..a8c869403 --- /dev/null +++ b/example/convex/functions/_helpers/project_access.ts @@ -0,0 +1,244 @@ +import { and, eq, type OrmWriter } from 'kitcn/orm'; +import type { AuthCtx } from '../../lib/crpc'; +import type { Insert, Select } from '../../shared/api'; +import type schema from '../schema'; +import { projectAccessTable } from '../schema'; + +type ProjectAccessCtx = Pick; +type ProjectAccessWriteCtx = { orm: OrmWriter }; +type ProjectRow = Select<'projects'>; + +/** + * Canonical owner of "which projects can this user see". + * + * Ownership lives on `projects.ownerId`, membership on `projectMembers`, and no + * index spans both. Answering the union by scanning `projects` and + * post-filtering in JS costs O(table) reads per page and puts every scanned row + * into the subscription's read set. Instead, every grant and revoke writes a + * `projectAccess` row here, and every read walks the + * `userId_archived_projectCreatedAt` index. + * + * Writes go through this module rather than through schema triggers on + * `projects`/`projectMembers`: async cascade continuation batches build their + * ORM from the raw `ctx.db` and drop triggers past the first batch, so a + * trigger-maintained copy would silently drift. Deletion is the one case that + * needs neither -- `projectAccess` cascades from both `projects` and `user`, and + * foreign-key cascades do still run in those batches. + */ + +/** + * The project's creation time as the number `projectAccess` sorts on. + * + * `Select<'projects'>.createdAt` is a `Date` hydrated from `_creationTime`, and + * `_creationTime` carries a sub-millisecond fraction that a `Date` cannot hold, + * so this key is millisecond-resolution. Two projects created in the same + * millisecond share a key and fall back to the access row's own creation order. + * That is one stable total order across pages; it only decides which of two + * same-millisecond projects sorts first. + */ +const sortKeyFor = (project: Pick): number => + project.createdAt.getTime(); + +/** + * Grant `userId` access to `project`, or refresh the row if it already exists. + * + * `isOwner` is derived from `project.ownerId` rather than passed in, so a caller + * cannot mislabel a row -- granting membership to the owner keeps them the owner + * instead of silently demoting them. Callers that grant during an ownership + * change must therefore pass the project as it will be after the change. + */ +export const grantProjectAccess = async ( + ctx: ProjectAccessWriteCtx, + input: { + project: Pick; + userId: string; + } +): Promise => { + const values: Insert<'projectAccess'> = { + userId: input.userId, + projectId: input.project.id, + projectCreatedAt: sortKeyFor(input.project), + archived: input.project.archived, + isOwner: input.project.ownerId === input.userId, + }; + + const existing = await ctx.orm.query.projectAccess.findFirst({ + where: { userId: input.userId, projectId: input.project.id }, + columns: { id: true }, + }); + + if (existing) { + await ctx.orm + .update(projectAccessTable) + .set(values) + .where(eq(projectAccessTable.id, existing.id)); + return; + } + + await ctx.orm.insert(projectAccessTable).values(values); +}; + +/** + * Drop `userId`'s access row for `projectId` unless they are still entitled. + * + * Re-derives entitlement from `projects.ownerId` and `projectMembers` instead of + * deleting unconditionally, so it stays correct whatever order a caller runs its + * steps in, and so a user who is both owner and member does not lose the project + * when only one of those two is taken away. A no-op when there is no row. + */ +export const revokeProjectAccess = async ( + ctx: ProjectAccessWriteCtx, + input: { projectId: string; userId: string } +): Promise => { + const project = await ctx.orm.query.projects.findFirst({ + where: { id: input.projectId }, + }); + + if (project) { + if (project.ownerId === input.userId) { + await grantProjectAccess(ctx, { project, userId: input.userId }); + return; + } + + const membership = await ctx.orm.query.projectMembers.findFirst({ + where: { projectId: input.projectId, userId: input.userId }, + columns: { id: true }, + }); + if (membership) { + await grantProjectAccess(ctx, { project, userId: input.userId }); + return; + } + } + + await ctx.orm + .delete(projectAccessTable) + .where( + and( + eq(projectAccessTable.projectId, input.projectId), + eq(projectAccessTable.userId, input.userId) + )! + ); +}; + +/** + * Mirror `projects.archived` onto every access row for the project. + * + * Bounded by the project's member count, which `addMember` already gates. + */ +export const syncProjectArchived = async ( + ctx: ProjectAccessWriteCtx, + input: { projectId: string; archived: boolean } +): Promise => { + await ctx.orm + .update(projectAccessTable) + .set({ archived: input.archived }) + .where(eq(projectAccessTable.projectId, input.projectId)); +}; + +/** + * Does this user have at least one non-archived project? + * + * The app-shell nav asks this on every route, so it must stay a single indexed + * read that cannot grow with the table. + */ +export const hasAnyProject = async ( + ctx: ProjectAccessCtx, + userId: string +): Promise => { + const access = await ctx.orm.query.projectAccess.findFirst({ + where: { userId, archived: false }, + columns: { id: true }, + }); + + return access !== null; +}; + +/** Every project the user can see, newest first, one index walk. */ +export const listAccessibleProjects = async ( + ctx: ProjectAccessCtx, + input: { + userId: string; + archived: boolean; + cursor: string | null; + limit: number; + } +) => { + const access = await ctx.orm.query.projectAccess.findMany({ + where: { userId: input.userId, archived: input.archived }, + orderBy: { projectCreatedAt: 'desc' }, + cursor: input.cursor, + limit: input.limit, + with: { project: true }, + }); + + const page = access.page + .map((row) => row.project) + .filter((project): project is ProjectRow => project !== null); + + return { + continueCursor: access.continueCursor, + isDone: access.isDone, + page, + }; +}; + +/** Non-archived projects the user can see, for the todo-form dropdown. */ +export const listProjectsForDropdown = async ( + ctx: ProjectAccessCtx, + input: { userId: string; limit: number } +): Promise<{ id: string; isOwner: boolean; name: string }[]> => { + const access = await ctx.orm.query.projectAccess.findMany({ + where: { userId: input.userId, archived: false }, + orderBy: { projectCreatedAt: 'desc' }, + limit: input.limit, + columns: { isOwner: true }, + with: { project: { columns: { id: true, name: true } } }, + }); + + return access + .map((row) => + row.project + ? { id: row.project.id, isOwner: row.isOwner, name: row.project.name } + : null + ) + .filter((row): row is NonNullable => row !== null) + .sort((a, b) => a.name.localeCompare(b.name)); +}; + +/** + * Rebuild every access row from `projects` and `projectMembers`. + * + * Deliberately a full scan of both tables: it exists to backfill a deployment + * seeded before `projectAccess` existed, and to let a test assert that the + * derived rows match their sources. Never call it from a request path. + */ +export const backfillProjectAccess = async ( + ctx: ProjectAccessWriteCtx, + options?: { limit?: number } +): Promise => { + const limit = options?.limit ?? 1000; + + const projects = await ctx.orm.query.projects.findMany({ limit }); + const byId = new Map(projects.map((project) => [project.id, project])); + + let written = 0; + + for (const project of projects) { + await grantProjectAccess(ctx, { project, userId: project.ownerId }); + written += 1; + } + + const memberships = await ctx.orm.query.projectMembers.findMany({ limit }); + + for (const membership of memberships) { + const project = byId.get(membership.projectId); + if (!project || project.ownerId === membership.userId) { + continue; + } + + await grantProjectAccess(ctx, { project, userId: membership.userId }); + written += 1; + } + + return written; +}; diff --git a/example/convex/functions/_helpers/project_existence.ts b/example/convex/functions/_helpers/project_existence.ts deleted file mode 100644 index 2e52006d7..000000000 --- a/example/convex/functions/_helpers/project_existence.ts +++ /dev/null @@ -1,26 +0,0 @@ -import type { AuthCtx } from '../../lib/crpc'; - -type ProjectExistenceCtx = Pick; - -export const hasAnyProject = async ( - ctx: ProjectExistenceCtx, - userId: string -): Promise => { - const owned = await ctx.orm.query.projects.findFirst({ - where: { ownerId: userId, archived: false }, - columns: { id: true }, - }); - if (owned) { - return true; - } - - const membership = await ctx.orm.query.projectMembers.findFirst({ - where: { - userId, - project: { archived: false }, - }, - columns: { id: true }, - }); - - return membership !== null; -}; diff --git a/example/convex/functions/aggregateDemo.ts b/example/convex/functions/aggregateDemo.ts index f7bebc6b5..6533e768c 100644 --- a/example/convex/functions/aggregateDemo.ts +++ b/example/convex/functions/aggregateDemo.ts @@ -1,6 +1,7 @@ import { eq } from 'kitcn/orm'; import * as z from 'zod'; import { authMutation, authQuery } from '../lib/crpc'; +import { grantProjectAccess } from './_helpers/project_access'; import { AGGREGATE_PARITY_DEFINITIONS, type AggregateParityDefinition, @@ -875,10 +876,12 @@ async function seedDemoData(ctx: AggregateDemoMutationCtx): Promise { archived: false, ownerId: ctx.userId, }) - .returning({ id: projectsTable.id }); + .returning(); projectIds.push(project.id); + await grantProjectAccess(ctx, { project, userId: ctx.userId }); + const [member] = await ctx.orm .insert(projectMembersTable) .values({ diff --git a/example/convex/functions/generated/procedure-names.gen.ts b/example/convex/functions/generated/procedure-names.gen.ts index 7f22ec9b0..cad6d3872 100644 --- a/example/convex/functions/generated/procedure-names.gen.ts +++ b/example/convex/functions/generated/procedure-names.gen.ts @@ -4,19 +4,19 @@ export const procedureNames = { "admin.ts": [{ column: 3, line: 21, name: "admin:checkUserAdminStatus" }, { column: 3, line: 133, name: "admin:getAllUsers" }, { column: 3, line: 237, name: "admin:getDashboardStats" }, { column: 3, line: 88, name: "admin:grantAdminByEmail" }, { column: 3, line: 43, name: "admin:updateUserRole" }], - "aggregateDemo.ts": [{ column: 54, line: 1167, name: "aggregateDemo:exerciseIdempotentTrigger" }, { column: 37, line: 1055, name: "aggregateDemo:getSnapshot" }, { column: 3, line: 1078, name: "aggregateDemo:runDirectOp" }, { column: 50, line: 1059, name: "aggregateDemo:toggleRandomFillReset" }], + "aggregateDemo.ts": [{ column: 54, line: 1174, name: "aggregateDemo:exerciseIdempotentTrigger" }, { column: 37, line: 1062, name: "aggregateDemo:getSnapshot" }, { column: 3, line: 1085, name: "aggregateDemo:runDirectOp" }, { column: 50, line: 1066, name: "aggregateDemo:toggleRandomFillReset" }], "authDemo.ts": [{ column: 38, line: 600, name: "authDemo:getAuthState" }, { column: 37, line: 580, name: "authDemo:getSnapshot" }, { column: 38, line: 641, name: "authDemo:runCoverage" }, { column: 3, line: 630, name: "authDemo:runScenario" }], "authDemoData.ts": [{ column: 3, line: 12, name: "authDemoData:getSessionByToken" }, { column: 3, line: 35, name: "authDemoData:getUserById" }, { column: 3, line: 60, name: "authDemoData:setUserBio" }], "items/queries.ts": [{ column: 3, line: 11, name: "items/queries:get" }, { column: 3, line: 6, name: "items/queries:list" }], "migrationDemo.ts": [{ column: 35, line: 87, name: "migrationDemo:cancel" }, { column: 35, line: 24, name: "migrationDemo:getStatus" }, { column: 3, line: 75, name: "migrationDemo:runDown" }, { column: 34, line: 63, name: "migrationDemo:runUp" }], "organization.ts": [{ column: 3, line: 367, name: "organization:acceptInvitation" }, { column: 3, line: 1116, name: "organization:addMember" }, { column: 3, line: 981, name: "organization:cancelInvitation" }, { column: 3, line: 1020, name: "organization:checkSlug" }, { column: 3, line: 174, name: "organization:createOrganization" }, { column: 3, line: 36, name: "organization:createPersonalOrganization" }, { column: 3, line: 544, name: "organization:deleteOrganization" }, { column: 3, line: 1088, name: "organization:getActiveMember" }, { column: 3, line: 595, name: "organization:getOrganization" }, { column: 3, line: 672, name: "organization:getOrganizationOverview" }, { column: 3, line: 889, name: "organization:inviteMember" }, { column: 3, line: 453, name: "organization:leaveOrganization" }, { column: 3, line: 750, name: "organization:listMembers" }, { column: 3, line: 134, name: "organization:listOrganizations" }, { column: 3, line: 830, name: "organization:listPendingInvitations" }, { column: 3, line: 1041, name: "organization:listUserInvitations" }, { column: 3, line: 93, name: "organization:listUserOrganizations" }, { column: 3, line: 398, name: "organization:rejectInvitation" }, { column: 3, line: 429, name: "organization:removeMember" }, { column: 3, line: 361, name: "organization:setActiveOrganization" }, { column: 3, line: 519, name: "organization:updateMemberRole" }, { column: 3, line: 240, name: "organization:updateOrganization" }], - "ormDemo.ts": [{ column: 45, line: 277, name: "ormDemo:clearPolymorphic" }, { column: 37, line: 263, name: "ormDemo:getSnapshot" }, { column: 40, line: 296, name: "ormDemo:runCoverage" }, { column: 44, line: 267, name: "ormDemo:seedPolymorphic" }], + "ormDemo.ts": [{ column: 45, line: 285, name: "ormDemo:clearPolymorphic" }, { column: 37, line: 271, name: "ormDemo:getSnapshot" }, { column: 40, line: 304, name: "ormDemo:runCoverage" }, { column: 44, line: 275, name: "ormDemo:seedPolymorphic" }], "plugins/ratelimit.ts": [{ column: 3, line: 20, name: "plugins/ratelimit:cleanup" }], "plugins/resend.ts": [{ column: 3, line: 649, name: "plugins/resend:callResendAPIWithBatch" }, { column: 3, line: 340, name: "plugins/resend:cancelEmail" }, { column: 3, line: 135, name: "plugins/resend:cleanupAbandonedEmails" }, { column: 3, line: 107, name: "plugins/resend:cleanupOldEmails" }, { column: 3, line: 278, name: "plugins/resend:createManualEmail" }, { column: 3, line: 400, name: "plugins/resend:get" }, { column: 3, line: 486, name: "plugins/resend:getAllContentByIds" }, { column: 3, line: 760, name: "plugins/resend:getEmailByResendId" }, { column: 3, line: 539, name: "plugins/resend:getEmailsByIds" }, { column: 3, line: 371, name: "plugins/resend:getStatus" }, { column: 3, line: 774, name: "plugins/resend:handleEmailEvent" }, { column: 3, line: 441, name: "plugins/resend:makeBatch" }, { column: 3, line: 564, name: "plugins/resend:markEmailsFailed" }, { column: 3, line: 606, name: "plugins/resend:onEmailComplete" }, { column: 3, line: 884, name: "plugins/resend:onEmailEvent" }, { column: 3, line: 178, name: "plugins/resend:sendEmail" }, { column: 3, line: 313, name: "plugins/resend:updateManualEmail" }], - "projects.ts": [{ column: 3, line: 432, name: "projects:addMember" }, { column: 3, line: 385, name: "projects:archive" }, { column: 3, line: 331, name: "projects:create" }, { column: 3, line: 229, name: "projects:get" }, { column: 52, line: 616, name: "projects:hasAny" }, { column: 3, line: 503, name: "projects:leave" }, { column: 3, line: 34, name: "projects:list" }, { column: 3, line: 575, name: "projects:listForDropdown" }, { column: 3, line: 479, name: "projects:removeMember" }, { column: 3, line: 406, name: "projects:restore" }, { column: 3, line: 521, name: "projects:transfer" }, { column: 3, line: 359, name: "projects:update" }], + "projects.ts": [{ column: 3, line: 436, name: "projects:addMember" }, { column: 3, line: 379, name: "projects:archive" }, { column: 3, line: 319, name: "projects:create" }, { column: 3, line: 217, name: "projects:get" }, { column: 52, line: 613, name: "projects:hasAny" }, { column: 3, line: 518, name: "projects:leave" }, { column: 3, line: 41, name: "projects:list" }, { column: 3, line: 609, name: "projects:listForDropdown" }, { column: 3, line: 489, name: "projects:removeMember" }, { column: 3, line: 405, name: "projects:restore" }, { column: 3, line: 541, name: "projects:transfer" }, { column: 3, line: 353, name: "projects:update" }], "public.ts": [{ column: 3, line: 7, name: "public:hello" }], "ratelimitDemo.ts": [{ column: 3, line: 486, name: "ratelimitDemo:consumeInteractive" }, { column: 3, line: 474, name: "ratelimitDemo:getInteractiveStatus" }, { column: 37, line: 440, name: "ratelimitDemo:getSnapshot" }, { column: 3, line: 498, name: "ratelimitDemo:resetInteractive" }, { column: 38, line: 526, name: "ratelimitDemo:runCoverage" }, { column: 3, line: 513, name: "ratelimitDemo:runCoverageProbe" }], - "seed.ts": [{ column: 47, line: 66, name: "seed:cleanupSeedData" }, { column: 3, line: 130, name: "seed:generateSamples" }, { column: 3, line: 174, name: "seed:generateSamplesBatch" }, { column: 36, line: 55, name: "seed:seed" }, { column: 3, line: 71, name: "seed:seedUsers" }], + "seed.ts": [{ column: 47, line: 67, name: "seed:cleanupSeedData" }, { column: 3, line: 131, name: "seed:generateSamples" }, { column: 3, line: 175, name: "seed:generateSamplesBatch" }, { column: 36, line: 56, name: "seed:seed" }, { column: 3, line: 72, name: "seed:seedUsers" }], "tags.ts": [{ column: 3, line: 52, name: "tags:create" }, { column: 3, line: 123, name: "tags:deleteTag" }, { column: 3, line: 21, name: "tags:list" }, { column: 3, line: 140, name: "tags:merge" }, { column: 3, line: 162, name: "tags:popular" }, { column: 3, line: 90, name: "tags:update" }], "todoComments.ts": [{ column: 3, line: 302, name: "todoComments:addComment" }, { column: 3, line: 475, name: "todoComments:cleanupOrphanedComments" }, { column: 3, line: 427, name: "todoComments:deleteComment" }, { column: 3, line: 115, name: "todoComments:getCommentThread" }, { column: 3, line: 34, name: "todoComments:getTodoComments" }, { column: 3, line: 237, name: "todoComments:getUserComments" }, { column: 3, line: 395, name: "todoComments:updateComment" }], "todoInternal.ts": [{ column: 3, line: 333, name: "todoInternal:archiveOldCompletedTodos" }, { column: 3, line: 636, name: "todoInternal:create" }, { column: 3, line: 686, name: "todoInternal:deleteTodo" }, { column: 3, line: 496, name: "todoInternal:generateWeeklyReport" }, { column: 3, line: 126, name: "todoInternal:getSystemStats" }, { column: 3, line: 597, name: "todoInternal:getUserWeeklyActivity" }, { column: 3, line: 36, name: "todoInternal:getUsersWithOverdueTodos" }, { column: 3, line: 451, name: "todoInternal:processDailySummaries" }, { column: 3, line: 370, name: "todoInternal:recalculateUserStats" }, { column: 3, line: 662, name: "todoInternal:update" }, { column: 3, line: 290, name: "todoInternal:updateOverduePriorities" }], diff --git a/example/convex/functions/ormDemo.ts b/example/convex/functions/ormDemo.ts index 79fcdbc41..ca370f545 100644 --- a/example/convex/functions/ormDemo.ts +++ b/example/convex/functions/ormDemo.ts @@ -1,5 +1,6 @@ import { eq, type InferInsertModel } from 'kitcn/orm'; import { authMutation, authQuery } from '../lib/crpc'; +import { grantProjectAccess } from './_helpers/project_access'; import type { MutationCtx, QueryCtx } from './generated/server'; import { ormPolymorphicEventTable, @@ -134,21 +135,27 @@ async function ensureDemoArtifacts(ctx: OrmDemoMutationCtx): Promise<{ columns: { id: true }, }); - const projectId = - existingProject?.id ?? - ( - await ctx.orm - .insert(projectsTable) - .values({ - name: `ORM polymorphic project ${now}`, - description: 'Created by orm demo coverage.', - isPublic: false, - archived: false, - ownerId: ctx.userId, - }) - .returning({ id: projectsTable.id }) - .execute() - )[0]?.id; + let projectId = existingProject?.id; + if (!projectId) { + const [createdProject] = await ctx.orm + .insert(projectsTable) + .values({ + name: `ORM polymorphic project ${now}`, + description: 'Created by orm demo coverage.', + isPublic: false, + archived: false, + ownerId: ctx.userId, + }) + .returning() + .execute(); + + await grantProjectAccess(ctx, { + project: createdProject, + userId: ctx.userId, + }); + + projectId = createdProject?.id; + } const existingTag = await ctx.orm.query.tags.findFirst({ where: { createdBy: ctx.userId }, diff --git a/example/convex/functions/projects.ts b/example/convex/functions/projects.ts index 41bab5603..3c610e132 100644 --- a/example/convex/functions/projects.ts +++ b/example/convex/functions/projects.ts @@ -3,7 +3,14 @@ import { CRPCError } from 'kitcn/server'; import * as z from 'zod'; import { authMutation, authQuery, optionalAuthQuery } from '../lib/crpc'; import type { Insert, Select } from '../shared/api'; -import { hasAnyProject } from './_helpers/project_existence'; +import { + grantProjectAccess, + hasAnyProject, + listAccessibleProjects, + listProjectsForDropdown, + revokeProjectAccess, + syncProjectArchived, +} from './_helpers/project_access'; import { projectMembersTable, projectsTable } from './schema'; // Schema for project list items @@ -153,34 +160,15 @@ export const list = optionalAuthQuery }; } - const memberships = await ctx.orm.query.projectMembers.findMany({ - where: { userId }, - limit: 1000, - columns: { projectId: true }, + // One index walk over the viewer's own access rows. The previous read model + // scanned `projects` and post-filtered owner-or-member in JS, so it cost + // O(table) per page and every scanned row joined this subscription. + const results = await listAccessibleProjects(ctx, { + userId, + archived: input.includeArchived === true, + cursor: input.cursor, + limit: input.limit, }); - const memberProjectIds = new Set(memberships.map((m) => m.projectId)); - - const results = await ctx.orm.query.projects - .select() - .orderBy({ createdAt: 'desc' }) - .filter(async (project: ProjectRow) => { - const isOwner = project.ownerId === userId; - const isMember = memberProjectIds.has(project.id); - - if (!(isOwner || isMember)) { - return false; - } - - if (input.includeArchived) { - return project.archived; - } - - return !project.archived; - }) - .paginate({ - cursor: input.cursor, - limit: input.limit, - }); const projectIds = results.page.map((p) => p.id); const statsByProject = await getProjectStats(projectIds); @@ -342,6 +330,8 @@ export const create = authMutation .values(values) .returning(); + await grantProjectAccess(ctx, { project, userId: ctx.userId }); + return project.id; }); @@ -398,6 +388,11 @@ export const archive = authMutation .update(projectsTable) .set({ archived: true }) .where(eq(projectsTable.id, input.projectId)); + + await syncProjectArchived(ctx, { + projectId: input.projectId, + archived: true, + }); }); export const restore = authMutation @@ -419,6 +414,11 @@ export const restore = authMutation .update(projectsTable) .set({ archived: false }) .where(eq(projectsTable.id, input.projectId)); + + await syncProjectArchived(ctx, { + projectId: input.projectId, + archived: false, + }); }); export const addMember = authMutation @@ -466,6 +466,8 @@ export const addMember = authMutation projectId: input.projectId, userId: userToAdd.id, }); + + await grantProjectAccess(ctx, { project, userId: userToAdd.id }); }); export const removeMember = authMutation @@ -495,6 +497,11 @@ export const removeMember = authMutation await ctx.orm .delete(projectMembersTable) .where(eq(projectMembersTable.id, member.id)); + + await revokeProjectAccess(ctx, { + projectId: input.projectId, + userId: input.userId, + }); }); export const leave = authMutation @@ -508,6 +515,11 @@ export const leave = authMutation await ctx.orm .delete(projectMembersTable) .where(eq(projectMembersTable.id, member.id)); + + await revokeProjectAccess(ctx, { + projectId: input.projectId, + userId: ctx.userId, + }); }); export const transfer = authMutation @@ -560,6 +572,21 @@ export const transfer = authMutation .update(projectsTable) .set({ ownerId: input.newOwnerId }) .where(eq(projectsTable.id, input.projectId)); + + // Both users keep access; only the `isOwner` flag moves. `grantProjectAccess` + // derives that flag from the project it is handed, so it has to see the + // post-transfer owner. Writing each row from the final state keeps this + // correct whatever order the steps above ran in, and re-granting an existing + // row is an update, not a duplicate. + const transferred = { ...project, ownerId: input.newOwnerId }; + await grantProjectAccess(ctx, { + project: transferred, + userId: input.newOwnerId, + }); + await grantProjectAccess(ctx, { + project: transferred, + userId: ctx.userId, + }); }); export const listForDropdown = authQuery @@ -572,45 +599,8 @@ export const listForDropdown = authQuery }) ) ) - .query(async ({ ctx }) => { - const userId = ctx.userId; - - const owned = await ctx.orm.query.projects.findMany({ - where: { ownerId: userId, archived: false }, - limit: 1000, - columns: { id: true, name: true }, - extras: { isOwner: true }, - }); - - const memberRows = await ctx.orm.query.projectMembers.findMany({ - where: { userId }, - limit: 1000, - columns: { projectId: true }, - }); - - const memberProjectIds = Array.from( - new Set(memberRows.map((row) => row.projectId)) - ); - - const memberProjects = memberProjectIds.length - ? await ctx.orm.query.projects.findMany({ - where: { id: { in: memberProjectIds }, archived: false }, - limit: memberProjectIds.length, - columns: { id: true, name: true }, - extras: { isOwner: false }, - }) - : []; - - const byId = new Map< - string, - { id: string; isOwner: boolean; name: string } - >(); - for (const project of memberProjects) byId.set(project.id, project); - for (const project of owned) byId.set(project.id, project); - - return Array.from(byId.values()).sort((a, b) => - a.name.localeCompare(b.name) - ); + .query(({ ctx }) => { + return listProjectsForDropdown(ctx, { userId: ctx.userId, limit: 1000 }); }); export const hasAny = authQuery.output(z.boolean()).query(({ ctx }) => { diff --git a/example/convex/functions/schema.ts b/example/convex/functions/schema.ts index 1908316bf..80a497f03 100644 --- a/example/convex/functions/schema.ts +++ b/example/convex/functions/schema.ts @@ -349,6 +349,15 @@ export const projectsTable = convexTable( index('isPublic').on(t.isPublic), index('archived').on(t.archived), index('ownerId').on(t.ownerId), + // `{ isPublic, archived }` and `{ ownerId, archived }` each tie two + // single-field indexes at the same planner score, and the stable sort hands + // the win to whichever was declared first -- leaving the other field as a + // JS post-filter over the whole partition. These compound indexes make both + // an exact match. They also serve `orderBy: { createdAt: 'desc' }`: every + // declared field is pinned by an equality, so Convex's implicit trailing + // `_creationTime` key orders the range and the scan returns the page. + index('isPublic_archived').on(t.isPublic, t.archived), + index('ownerId_archived').on(t.ownerId, t.archived), aggregateIndex('by_owner').on(t.ownerId), searchIndex('search_name_description') .on(t.name) @@ -356,6 +365,61 @@ export const projectsTable = convexTable( ] ); +/** + * Denormalized "which projects can this user see", one row per (user, project). + * + * `projects.list` has to order owner-or-member projects by project creation + * time. Ownership lives on `projects.ownerId` and membership lives on + * `projectMembers`, and no index spans both, so the question used to be answered + * by scanning `projects` and post-filtering in JS -- O(table) reads, with every + * scanned row joining the subscription's read set. This table answers it with a + * single index walk instead. + * + * Maintained by `_helpers/project_access.ts`, which owns every write that grants + * or revokes access, rather than by schema triggers: async cascade continuation + * batches build their ORM from the raw `ctx.db` and drop triggers past the first + * batch, so a trigger-maintained copy would drift. Deletion needs neither -- both + * foreign keys cascade, and cascades still run in those batches. + */ +export const projectAccessTable = convexTable( + 'projectAccess', + { + createdAt: timestamp().notNull().defaultNow(), + userId: text() + .references(() => userTable.id, { onDelete: 'cascade' }) + .notNull(), + projectId: text() + .references(() => projectsTable.id, { onDelete: 'cascade' }) + .notNull(), + /** + * The project's creation time, as a plain number, so it can be an index + * field. A `timestamp()` here would be redundant: the value is only ever + * compared, never read back as a date. + * + * Millisecond resolution. `_creationTime` carries a sub-millisecond + * fraction, but it reaches this column through a hydrated `Date`, which + * cannot hold it, so two projects created in the same millisecond share a + * key and are separated by the access row's own creation order. + */ + projectCreatedAt: integer().notNull(), + /** Mirrors `projects.archived` so the toggle is part of the index range. */ + archived: boolean().notNull(), + isOwner: boolean().notNull(), + }, + (t) => [ + // `userId` and `archived` pinned by equality, then `projectCreatedAt` is the + // next index field, so `orderBy: { projectCreatedAt: 'desc' }` is served by + // walking the range backwards. Reads become O(page). + index('userId_archived_projectCreatedAt').on( + t.userId, + t.archived, + t.projectCreatedAt + ), + uniqueIndex('userId_projectId').on(t.userId, t.projectId), + index('projectId').on(t.projectId), + ] +); + // -------------------- // Tag Model // -------------------- @@ -639,6 +703,7 @@ export const tables = { subscriptions: subscriptionsTable, todos: todosTable, projects: projectsTable, + projectAccess: projectAccessTable, tags: tagsTable, todoComments: todoCommentsTable, projectMembers: projectMembersTable, @@ -756,6 +821,10 @@ const schema = defineSchema(tables, { to: r.projects.id.through(r.projectMembers.projectId), alias: 'ProjectMembers', }), + projectAccess: r.many.projectAccess({ + from: r.user.id, + to: r.projectAccess.userId, + }), todoComments: r.many.todoComments({ from: r.user.id, to: r.todoComments.userId, @@ -824,12 +893,26 @@ const schema = defineSchema(tables, { to: r.todoComments.todoId, }), }, + projectAccess: { + project: r.one.projects({ + from: r.projectAccess.projectId, + to: r.projects.id, + }), + user: r.one.user({ + from: r.projectAccess.userId, + to: r.user.id, + }), + }, projects: { owner: r.one.user({ from: r.projects.ownerId, to: r.user.id, alias: 'ProjectOwner', }), + access: r.many.projectAccess({ + from: r.projects.id, + to: r.projectAccess.projectId, + }), todos: r.many.todos({ from: r.projects.id, to: r.todos.projectId, diff --git a/example/convex/functions/seed.ts b/example/convex/functions/seed.ts index 042b04903..a6954f103 100644 --- a/example/convex/functions/seed.ts +++ b/example/convex/functions/seed.ts @@ -3,6 +3,7 @@ import * as z from 'zod'; import { createUser } from '../lib/auth/auth-helpers'; import { authAction, privateMutation } from '../lib/crpc'; import { getEnv } from '../lib/get-env'; +import { grantProjectAccess } from './_helpers/project_access'; import { createSeedCaller, createSeedHandler } from './generated/seed.runtime'; import { projectsTable, @@ -367,7 +368,7 @@ export const generateSamplesBatch = privateMutation const isPublic = Math.random() > 0.7; // 30% public const isArchived = Math.random() > 0.9; // 10% archived - const [{ id: projectId }] = await ctx.orm + const [project] = await ctx.orm .insert(projectsTable) .values({ name, @@ -376,7 +377,10 @@ export const generateSamplesBatch = privateMutation isPublic, archived: isArchived, }) - .returning({ id: projectsTable.id }); + .returning(); + const projectId = project.id; + + await grantProjectAccess(ctx, { project, userId: input.userId }); created++; From 9289171a77405c3628b1d8ad243177be3a74c1c1 Mon Sep 17 00:00:00 2001 From: MikeyZhang75 Date: Sun, 6 Sep 2026 10:18:57 +0800 Subject: [PATCH 2/7] docs(plans): record PR #452 in the 443 task plan --- ...026-09-05-443-projects-list-read-bounds.md | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md index 54659fc5a..ca4522e4e 100644 --- a/docs/plans/2026-09-05-443-projects-list-read-bounds.md +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -80,8 +80,8 @@ Boundaries: deployment (only `.env.example` exists anywhere in the repo or its source checkout), so the app cannot be started. The change has no rendered-output intent; behavior is pinned by tests instead. -- GitHub issue sync: not posted. Posting is outward-facing and the user's only - standing instruction was "no PR"; offered in the handoff instead. +- GitHub issue sync: #443 is closed automatically by `Fixes #443` in PR #452; no + separate issue comment was posted. - Non-goals: the ORM union-source id-IN lowering, the `.filter()` raw-row type lie, and the async-cascade trigger drop. All recorded as follow-ups. @@ -97,10 +97,12 @@ Blocked condition: Task state: - task_type: bug - task_complexity: non-trivial -- current_phase: closeout +- current_phase: shipped - current_phase_status: done - next_phase: final response - goal_status: complete +- pr: https://github.com/udecode/kitcn/pull/452 +- branch: fix/project-list-read-bounds Current verdict: - verdict: partially valid, fixed @@ -186,7 +188,7 @@ Start Gates: | Skill analysis before edits | yes | task + autogoal + autoreview loaded; no others earned their keep | | Active goal checked or created | yes | this plan | | Source of truth read before edits | yes | gh issue view 443 + projects.ts + schema.ts read first | -| Exact per-PR task ownership | no | N/A: user declined PRs | +| Exact per-PR task ownership | yes | https://github.com/udecode/kitcn/pull/452 | | GitHub comments and attachments read | yes | issue has zero comments (gh --json comments returned []) | | Video transcript evidence required | no | N/A: no video in source | | Pre-solution issue challenge required | yes | recorded above; verdict partially valid | @@ -198,9 +200,9 @@ Start Gates: | Branch decision for code-changing task | yes | already on dedicated branch issue-443-task | | Release artifact decision | no | N/A: example + test-infra only; .changeset/config.json ignores example, nothing under packages/ changed | | Browser tool decision for browser surface | yes | waived: no Convex deployment exists, so the app cannot start | -| Commit / PR expectation decision | no | N/A: explicit user decline โ€” "Do not create PR under any circumstances, unless user prompts to" | -| Task-style PR body decision | no | N/A: no PR | -| Task-plan PR body evidence | no | N/A: no PR | +| Commit / PR expectation decision | yes | user prompted for a PR; committed, pushed, opened #452 | +| Task-style PR body decision | yes | PR #270 emoji format used | +| Task-plan PR body evidence | yes | body carries `๐Ÿงญ Task plan:`; plan present at PR head; names #452 | | GitHub issue sync expectation decision | no | not posted; outward-facing, offered to the user instead | | Output budget strategy recorded | yes | recorded above | @@ -274,7 +276,7 @@ Completion Gates: | Gate | Applies | Required action | Evidence | |------|---------|-----------------|----------| | Named verification threshold | yes | Ran | `bunx vitest run convex/orm/example-project-access-reads.test.ts` โ€” 13 pass; cost 4 reads at 10 and at 400 noise rows | -| Exact per-PR task ownership | no | N/A | user declined PRs | +| Exact per-PR task ownership | yes | Recorded | https://github.com/udecode/kitcn/pull/452 | | Pre-solution issue challenge verdict | yes | Recorded | partially valid; claim (b) invalid, no code written for it | | Repro escalation ladder | yes | Recorded | source-level repro captured; browser N/A (no deployment) | | Bug reproduced before fix | yes | Captured | scanned 40 / page 0, and member-only project invisible at page 0 of 31 | @@ -294,12 +296,12 @@ Completion Gates: | High-risk mini gate | yes | Recorded | see High-risk note below | | Agent-native review for agent/tooling changes | no | N/A | no agent-native surface touched | | Local install corruption suspected | no | N/A | no corruption-shaped failure appeared | -| Commit created | no | N/A | explicit user decline of PRs; work left in the worktree for review | -| PR create or update | no | N/A | explicit user decline | -| Task-style PR body verified | no | N/A | no PR | -| PR task evidence verified | no | N/A | no PR | -| PR proof image hosting | no | N/A | no PR | -| GitHub issue sync-back | no | Not posted | outward-facing publish; user's standing instruction covered PRs only, so offered in the handoff instead | +| Commit created | yes | Created | 9f0275f1 on fix/project-list-read-bounds | +| PR create or update | yes | Created | https://github.com/udecode/kitcn/pull/452 (base main) | +| Task-style PR body verified | yes | Verified | `gh pr view 452 --json body` โ€” 4 bold emoji sections, correct table header, no self-link | +| PR task evidence verified | yes | Verified | plan line in body, plan at PR head, names #452 | +| PR proof image hosting | no | N/A | no images in body | +| GitHub issue sync-back | no | Not posted | #443 is auto-linked by `Fixes #443` in PR #452; a separate comment is outward-facing and was not requested | | Final handoff contract | yes | Filled | see Final handoff contract | | Final lint | yes | Ran | `bun lint:fix` clean; `biome check` reports no fixes needed | | Output budget discipline | yes | Recorded | workflow reports read from journal, command output filtered | @@ -313,7 +315,7 @@ Phase / pass table: | Intake and source read | done | issue + code read, repro captured | implementation | | Implementation | done | schema table + helper + call sites + harness fix | verification | | Verification | done | see Verification evidence | closeout | -| Commit / PR / GitHub sync | N/A | user explicitly declined PRs ("Do not create PR under any circumstances, unless user prompts to"); work left uncommitted in the worktree | final response | +| Commit / PR / GitHub sync | done | commit 9f0275f1, pushed, PR #452 opened onto main | final response | | Closeout | done | autoreview clean, all gates run | final response | Findings: @@ -461,9 +463,9 @@ Source-listed case matrix: | Unauth branch post-filter | NOT in the issue | `index('isPublic_archived')` | `archived` post-filtered | exact index match | schema change | fixed | Final handoff contract: -- Commit line: none โ€” user explicitly declined PRs; changes left in the worktree -- PR line: N/A -- Issue line: not posted; offered to the user +- Commit line: 9f0275f1 on `fix/project-list-read-bounds` +- PR line: https://github.com/udecode/kitcn/pull/452 +- Issue line: #443 auto-linked via `Fixes #443`; no separate comment posted - Confidence line: 95-100% on the read bound and the correctness fix - Flow table: - Reproduced: tests yes (scanned 40 / page 0; member-only invisible), browser N/A @@ -485,7 +487,7 @@ Final handoff contract: into an example-only bug - Verified: `bun typecheck` 5/5, `bun run test` 1400 bun + 992 vitest / 0 fail, `bun lint:fix` clean, autoreview clean -- PR body verified: N/A โ€” no PR +- PR body verified: `gh pr view 452 --json body` โ€” task-style format confirmed Task-style PR body contract: - Preserve any existing `` block. If a changeset is @@ -509,9 +511,9 @@ Task-style PR body contract: of that output. Final handoff / sync: -- Commit: none (explicit user decline) -- PR: N/A -- Issue: not posted; offered +- Commit: 9f0275f1 +- PR: https://github.com/udecode/kitcn/pull/452 +- Issue: linked from the PR - Browser proof: waived, no deployment - Caveats: see Open risks @@ -521,7 +523,7 @@ Timeline: Reboot status: | Question | Answer | |----------|--------| -| Where am I? | Closeout complete; awaiting the user on commit/PR/issue sync | +| Where am I? | Shipped as PR #452 | | Where am I going? | Final response only | | What is the goal? | Make `projects.list` reads independent of table size and narrow its read set | | What have I learned? | See Findings โ€” three defects, only one of which the issue reported | From 0376934447930f1286aeab0a9cd8706006ae6176 Mon Sep 17 00:00:00 2001 From: MikeyZhang75 Date: Sun, 6 Sep 2026 10:35:09 +0800 Subject: [PATCH 3/7] fix(example): ship the projectAccess backfill as a real migration `projectAccess` started empty on any deployment that already held projects, so switching `list`, `listForDropdown` and `hasAny` onto it made all three return nothing. `backfillProjectAccess` existed but was referenced only by its test and `migrations/manifest.ts` was empty, so there was no callable path to populate it. Add `20260906_022807_backfill_project_access`, scaffolded with `kitcn migrate create` and registered in the manifest. It walks `projects` in batches and re-derives each project's rows; `grantProjectAccess` updates rather than inserts, so re-running is safe. `down` clears them. The runner pages `ctx.db.query(table)`, so `migrateOne` receives the RAW Convex document -- `_id`, not `id`, and no hydrated `createdAt`. Same trap that made the old `.filter()` membership check dead. Re-read the row through the ORM instead of handing that shape to a helper expecting the public one. Extract `syncProjectAccessForProject` as the single owner of "what should this project's access rows look like", shared by the migration and `backfillProjectAccess`, so a deployment repaired by the migration and one rebuilt in a test cannot drift. This also fixes a latent bug in the old backfill, which capped memberships globally at 1000 rather than per project. Test drives the migration's own `migrateOne` over raw docs: reads are empty before, correct after, re-running does not duplicate, `down` empties. Verified armed by neutering the migration and watching it fail. --- .../orm/example-project-access-reads.test.ts | 88 +++++++++++++++++-- ...026-09-05-443-projects-list-read-bounds.md | 23 +++-- .../functions/_helpers/project_access.ts | 78 +++++++++++----- ...20260906_022807_backfill_project_access.ts | 63 +++++++++++++ .../convex/functions/migrations/manifest.ts | 5 +- 5 files changed, 221 insertions(+), 36 deletions(-) create mode 100644 example/convex/functions/migrations/20260906_022807_backfill_project_access.ts diff --git a/convex/orm/example-project-access-reads.test.ts b/convex/orm/example-project-access-reads.test.ts index 985638a0a..93bb72bc3 100644 --- a/convex/orm/example-project-access-reads.test.ts +++ b/convex/orm/example-project-access-reads.test.ts @@ -2,12 +2,15 @@ import { eq } from 'kitcn/orm'; import { expect, test } from 'vitest'; import { backfillProjectAccess, + clearProjectAccess, grantProjectAccess, hasAnyProject, listAccessibleProjects, + listProjectsForDropdown, revokeProjectAccess, syncProjectArchived, } from '../../example/convex/functions/_helpers/project_access'; +import { migration as backfillMigration } from '../../example/convex/functions/migrations/20260906_022807_backfill_project_access'; import schema, { projectAccessTable, projectMembersTable, @@ -569,20 +572,87 @@ test('backfill reconstructs the same access rows the write path maintains', asyn expect(maintained).toHaveLength(3); // Wipe the derived table, as a pre-`projectAccess` deployment would have - // it. A raw delete, not `revokeProjectAccess`, which would re-derive the - // rows straight back. - for (const row of await ctx.orm.query.projectAccess.findMany({ - limit: 100, - })) { - await ctx.orm - .delete(projectAccessTable) - .where(eq(projectAccessTable.id, row.id)); + // it. `clearProjectAccess`, not `revokeProjectAccess`, which would re-derive + // the rows straight back. + for (const projectId of [project.id, archivedProject.id]) { + await clearProjectAccess(ctx, { projectId }); } expect(await snapshot()).toEqual([]); await backfillProjectAccess(ctx); expect(await snapshot()).toEqual(maintained); - void archivedProject; + }); +}); + +/** + * The deploy case the read switch depends on: a deployment that already holds + * projects and memberships has no access rows at all, so `list`, `hasAny` and + * `listForDropdown` return nothing until the backfill migration runs. + * + * Drives the migration's own `migrateOne` over raw `ctx.db` documents, which is + * exactly what the runner pages in. + */ +test('backfill migration populates access for pre-existing data', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const member = await makeUser(ctx, 'member'); + + const shared = await makeProject(ctx, { name: 'shared', ownerId: owner }); + await addMember(ctx, { project: shared, userId: member }); + const solo = await makeProject(ctx, { name: 'solo', ownerId: owner }); + + // Roll the deployment back to its pre-`projectAccess` state. + for (const projectId of [shared.id, solo.id]) { + await clearProjectAccess(ctx, { projectId }); + } + + const listFor = (userId: string) => + listAccessibleProjects(ctx, { + userId, + archived: false, + cursor: null, + limit: 20, + }); + + // This is the regression the reviewer flagged: reads go dark. + expect((await listFor(owner)).page).toEqual([]); + expect((await listFor(member)).page).toEqual([]); + await expect(hasAnyProject(ctx, owner)).resolves.toBe(false); + + const rawProjects = await ctx.db.query('projects').collect(); + expect(rawProjects).toHaveLength(2); + for (const doc of rawProjects) { + await backfillMigration.up.migrateOne(ctx as never, doc as never); + } + + expect((await listFor(owner)).page.map((p) => p.name).sort()).toEqual([ + 'shared', + 'solo', + ]); + expect((await listFor(member)).page.map((p) => p.name)).toEqual(['shared']); + await expect(hasAnyProject(ctx, owner)).resolves.toBe(true); + await expect(hasAnyProject(ctx, member)).resolves.toBe(true); + expect( + (await listProjectsForDropdown(ctx, { userId: member, limit: 100 })).map( + (row) => [row.name, row.isOwner] + ) + ).toEqual([['shared', false]]); + + // Re-running must not duplicate rows. + for (const doc of rawProjects) { + await backfillMigration.up.migrateOne(ctx as never, doc as never); + } + expect( + await ctx.orm.query.projectAccess.findMany({ limit: 100 }) + ).toHaveLength(3); + + // `down` takes it back to empty. + for (const doc of rawProjects) { + await backfillMigration.down?.migrateOne(ctx as never, doc as never); + } + expect(await ctx.orm.query.projectAccess.findMany({ limit: 100 })).toEqual( + [] + ); }); }); diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md index ca4522e4e..3c6f5a6e4 100644 --- a/docs/plans/2026-09-05-443-projects-list-read-bounds.md +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -421,7 +421,18 @@ Implementation notes: - None yet. Review fixes: -- None yet. +- PR #452 thread PRRT_kwDOPTlS686foid6 (@chatgpt-codex-connector, P1): triaged + valid. `projectAccess` started empty on any deployment holding existing + projects, so `list`, `listForDropdown` and `hasAny` all returned nothing. + Added migration `20260906_022807_backfill_project_access` over `projects`, + registered in the manifest, plus `syncProjectAccessForProject` as the single + owner shared by the migration and `backfillProjectAccess`, and + `clearProjectAccess` for `down`. Test drives the migration's own `migrateOne` + over raw `ctx.db` docs; verified armed by neutering the migration and watching + it fail. Correction: my earlier note that this needed a Convex deployment was + wrong โ€” `runMigrationCreate` is pure filesystem and scaffolded offline. + The migration doc is the RAW Convex document (`_id`, no `id`, no hydrated + `createdAt`), same trap as `.filter()`, so it is re-read through the ORM. Error attempts: | Error / failed attempt | Count | Next different move | Resolution | @@ -535,10 +546,12 @@ Open risks: worktree. `kitcn codegen` did run and updated `example/convex/functions/generated/procedure-names.gen.ts`. Anyone with a deployment should run `bun --cwd example run codegen` before deploying. -- `backfillProjectAccess` is tested but not wired to a callable migration. The - migrations manifest is CLI-generated ("Do not edit manually") and - `kitcn migrate create` needs a deployment. An existing deployment shows empty - project lists until it is run; the example app is normally re-seeded. +- RESOLVED (PR #452 review, @chatgpt-codex-connector P1): the backfill is now a + real migration, `20260906_022807_backfill_project_access`, registered in + `migrations/manifest.ts`. My earlier claim that `kitcn migrate create` needs a + deployment was wrong โ€” `runMigrationCreate` is pure filesystem work and the + CLI scaffolded it offline. Pinned by a test that drives the migration's own + `migrateOne` over raw `ctx.db` docs and asserts reads go from empty to correct. - Pagination cursors change shape (they now page `projectAccess`, not `projects`). A cursor issued before the deploy is not valid after it. - Sort key is millisecond-resolution, so two projects created in the same diff --git a/example/convex/functions/_helpers/project_access.ts b/example/convex/functions/_helpers/project_access.ts index a8c869403..b2e13038f 100644 --- a/example/convex/functions/_helpers/project_access.ts +++ b/example/convex/functions/_helpers/project_access.ts @@ -5,7 +5,15 @@ import type schema from '../schema'; import { projectAccessTable } from '../schema'; type ProjectAccessCtx = Pick; -type ProjectAccessWriteCtx = { orm: OrmWriter }; +/** + * Any context that can write through the ORM. + * + * Exported because the backfill migration has to bridge to it: the migration + * runner types its `ctx.orm` off the resolved relations config rather than the + * schema chain, so the two describe the same runtime object under different + * type arguments. + */ +export type ProjectAccessWriteCtx = { orm: OrmWriter }; type ProjectRow = Select<'projects'>; /** @@ -205,39 +213,67 @@ export const listProjectsForDropdown = async ( .sort((a, b) => a.name.localeCompare(b.name)); }; +/** Drop every access row for a project, without re-deriving. */ +export const clearProjectAccess = async ( + ctx: ProjectAccessWriteCtx, + input: { projectId: string } +): Promise => { + await ctx.orm + .delete(projectAccessTable) + .where(eq(projectAccessTable.projectId, input.projectId)); +}; + /** - * Rebuild every access row from `projects` and `projectMembers`. + * Rebuild one project's access rows from `projects.ownerId` and its members. * - * Deliberately a full scan of both tables: it exists to backfill a deployment - * seeded before `projectAccess` existed, and to let a test assert that the - * derived rows match their sources. Never call it from a request path. + * The single owner of "what should this project's access rows look like". + * Both the backfill migration and `backfillProjectAccess` go through it, so a + * deployment repaired by the migration and one rebuilt in a test cannot drift + * apart. Bounded by the project's member count. */ -export const backfillProjectAccess = async ( +export const syncProjectAccessForProject = async ( ctx: ProjectAccessWriteCtx, + project: Pick, options?: { limit?: number } ): Promise => { - const limit = options?.limit ?? 1000; - - const projects = await ctx.orm.query.projects.findMany({ limit }); - const byId = new Map(projects.map((project) => [project.id, project])); + await grantProjectAccess(ctx, { project, userId: project.ownerId }); + let written = 1; - let written = 0; + const members = await ctx.orm.query.projectMembers.findMany({ + where: { projectId: project.id }, + limit: options?.limit ?? 1000, + columns: { userId: true }, + }); - for (const project of projects) { - await grantProjectAccess(ctx, { project, userId: project.ownerId }); + for (const member of members) { + if (member.userId === project.ownerId) { + continue; + } + await grantProjectAccess(ctx, { project, userId: member.userId }); written += 1; } - const memberships = await ctx.orm.query.projectMembers.findMany({ limit }); + return written; +}; - for (const membership of memberships) { - const project = byId.get(membership.projectId); - if (!project || project.ownerId === membership.userId) { - continue; - } +/** + * Rebuild every access row in the deployment. + * + * Deliberately a full scan of `projects`: it exists so a test can assert the + * derived rows match their sources. The deploy path is the backfill migration, + * which walks the same table in bounded batches and calls the same per-project + * sync. Never call this from a request path. + */ +export const backfillProjectAccess = async ( + ctx: ProjectAccessWriteCtx, + options?: { limit?: number } +): Promise => { + const limit = options?.limit ?? 1000; + const projects = await ctx.orm.query.projects.findMany({ limit }); - await grantProjectAccess(ctx, { project, userId: membership.userId }); - written += 1; + let written = 0; + for (const project of projects) { + written += await syncProjectAccessForProject(ctx, project, { limit }); } return written; diff --git a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts new file mode 100644 index 000000000..73b70afd4 --- /dev/null +++ b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts @@ -0,0 +1,63 @@ +import { + clearProjectAccess, + type ProjectAccessWriteCtx, + syncProjectAccessForProject, +} from '../_helpers/project_access'; +import { defineMigration } from '../generated/migrations.gen'; + +/** + * The migration runner types `ctx.orm` off the resolved relations config, while + * the helpers type theirs off the schema chain. Same runtime object, different + * type argument, so bridge it once here instead of widening the helpers. + */ +const writeCtx = (ctx: { orm: unknown }): ProjectAccessWriteCtx => + ctx as ProjectAccessWriteCtx; + +/** + * Populate `projectAccess` for data that predates it. + * + * `projects.list`, `listForDropdown` and `hasAny` all read the derived table, so + * on a deployment that already holds projects it starts empty and every one of + * them returns nothing until this runs. + * + * Walks `projects` in batches and re-derives each project's rows from + * `ownerId` plus its memberships. `grantProjectAccess` updates an existing row + * rather than inserting a second one, so re-running is safe. + */ +export const migration = defineMigration({ + id: '20260906_022807_backfill_project_access', + description: 'backfill project access', + up: { + table: 'projects', + migrateOne: async (ctx, doc) => { + // The runner pages `ctx.db.query(table)`, so `doc` is the raw Convex + // document: it carries `_id`, not the public `id`, and `_creationTime`, + // not a hydrated `createdAt`. Read the row back through the ORM rather + // than handing this shape to a helper that expects the public one. + const projectId = (doc as { _id?: string })._id; + if (!projectId) { + return; + } + + const project = await ctx.orm.query.projects.findFirst({ + where: { id: projectId }, + }); + if (!project) { + return; + } + + await syncProjectAccessForProject(writeCtx(ctx), project); + }, + }, + down: { + table: 'projects', + migrateOne: async (ctx, doc) => { + const projectId = (doc as { _id?: string })._id; + if (!projectId) { + return; + } + + await clearProjectAccess(writeCtx(ctx), { projectId }); + }, + }, +}); diff --git a/example/convex/functions/migrations/manifest.ts b/example/convex/functions/migrations/manifest.ts index cc8f15183..6f0a4cc0c 100644 --- a/example/convex/functions/migrations/manifest.ts +++ b/example/convex/functions/migrations/manifest.ts @@ -3,5 +3,8 @@ // Do not edit manually. import { defineMigrationSet } from 'kitcn/orm'; +import { migration as migration_0 } from './20260906_022807_backfill_project_access'; -export const migrations = defineMigrationSet([]); +export const migrations = defineMigrationSet([ + migration_0, +]); From f1387b4cccf72ca7e851f0b961fc4318642ee7cc Mon Sep 17 00:00:00 2001 From: MikeyZhang75 Date: Sun, 6 Sep 2026 11:59:35 +0800 Subject: [PATCH 4/7] fix(example): back fill project access from both source tables The backfill read each project's memberships in one `limit: 1000` call. Nothing caps a project's member count -- `addMember` only rejects the owner, a duplicate, and a non-owner caller -- and `findMany` truncates silently rather than throwing (probe: 5 rows, `limit: 3`, returned 3, no error). Members past the bound would get no access row, and `list`, `hasAny` and `listForDropdown` would hide a project they belong to, permanently and with the migration reporting success. Split the backfill into one migration per source table: owners over `projects`, memberships over `projectMembers`. The runner pages each table itself, so neither is bounded by a limit this code picks. That shape is also what keeps it correct at runtime. A cursor loop inside `migrateOne` would issue a second `.paginate()` inside the runner's own paginated read, which real Convex rejects with `MultiplePaginatedDatabaseQueries` while convex-test does not model it -- it would have passed tests and failed in prod. `backfillProjectAccess` is now explicitly test-only and reads `limit + 1` so it throws on truncation instead of quietly backfilling a prefix. Tests drive both migrations over raw `ctx.db` docs and cover every member row; both were verified armed by neutering each migration in turn. --- .../orm/example-project-access-reads.test.ts | 103 ++++++++++++++++-- ...026-09-05-443-projects-list-read-bounds.md | 17 +++ .../functions/_helpers/project_access.ts | 100 ++++++++++++----- ...20260906_022807_backfill_project_access.ts | 25 +++-- ..._035251_backfill_project_access_members.ts | 48 ++++++++ .../convex/functions/migrations/manifest.ts | 2 + 6 files changed, 247 insertions(+), 48 deletions(-) create mode 100644 example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts diff --git a/convex/orm/example-project-access-reads.test.ts b/convex/orm/example-project-access-reads.test.ts index 93bb72bc3..262ed88d4 100644 --- a/convex/orm/example-project-access-reads.test.ts +++ b/convex/orm/example-project-access-reads.test.ts @@ -10,7 +10,8 @@ import { revokeProjectAccess, syncProjectArchived, } from '../../example/convex/functions/_helpers/project_access'; -import { migration as backfillMigration } from '../../example/convex/functions/migrations/20260906_022807_backfill_project_access'; +import { migration as backfillOwnersMigration } from '../../example/convex/functions/migrations/20260906_022807_backfill_project_access'; +import { migration as backfillMembersMigration } from '../../example/convex/functions/migrations/20260906_035251_backfill_project_access_members'; import schema, { projectAccessTable, projectMembersTable, @@ -620,11 +621,23 @@ test('backfill migration populates access for pre-existing data', async () => { expect((await listFor(member)).page).toEqual([]); await expect(hasAnyProject(ctx, owner)).resolves.toBe(false); + // Each migration walks its own source table, exactly as the runner pages it. const rawProjects = await ctx.db.query('projects').collect(); + const rawMembers = await ctx.db.query('projectMembers').collect(); expect(rawProjects).toHaveLength(2); - for (const doc of rawProjects) { - await backfillMigration.up.migrateOne(ctx as never, doc as never); - } + expect(rawMembers).toHaveLength(1); + const runBackfill = async () => { + for (const doc of rawProjects) { + await backfillOwnersMigration.up.migrateOne(ctx as never, doc as never); + } + for (const doc of rawMembers) { + await backfillMembersMigration.up.migrateOne( + ctx as never, + doc as never + ); + } + }; + await runBackfill(); expect((await listFor(owner)).page.map((p) => p.name).sort()).toEqual([ 'shared', @@ -640,19 +653,91 @@ test('backfill migration populates access for pre-existing data', async () => { ).toEqual([['shared', false]]); // Re-running must not duplicate rows. - for (const doc of rawProjects) { - await backfillMigration.up.migrateOne(ctx as never, doc as never); - } + await runBackfill(); expect( await ctx.orm.query.projectAccess.findMany({ limit: 100 }) ).toHaveLength(3); - // `down` takes it back to empty. + // `down` takes it back to empty, reverse order. + for (const doc of rawMembers) { + await backfillMembersMigration.down?.migrateOne( + ctx as never, + doc as never + ); + } for (const doc of rawProjects) { - await backfillMigration.down?.migrateOne(ctx as never, doc as never); + await backfillOwnersMigration.down?.migrateOne( + ctx as never, + doc as never + ); } expect(await ctx.orm.query.projectAccess.findMany({ limit: 100 })).toEqual( [] ); }); }); + +/** + * Nothing caps a project's member count -- `projects.addMember` only rejects the + * owner, a duplicate, and a non-owner caller. A backfill that read a project's + * memberships in one bounded call would silently omit everyone past the bound + * and leave them permanently unable to see the project. + * + * The membership migration walks `projectMembers` itself, one row per + * `migrateOne`, so its coverage is the runner's paging rather than any limit + * this code chooses. + */ +test('membership backfill covers every member row, not a capped page', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const project = await makeProject(ctx, { name: 'crowded', ownerId: owner }); + + const members: string[] = []; + for (let index = 0; index < 12; index += 1) { + const member = await makeUser(ctx, `m${index}`); + await addMember(ctx, { project, userId: member }); + members.push(member); + } + + await clearProjectAccess(ctx, { projectId: project.id }); + + const rawMembers = await ctx.db.query('projectMembers').collect(); + expect(rawMembers).toHaveLength(members.length); + for (const doc of rawMembers) { + await backfillMembersMigration.up.migrateOne(ctx as never, doc as never); + } + + // Every member, including the last one written, can see the project again. + for (const member of members) { + const results = await listAccessibleProjects(ctx, { + userId: member, + archived: false, + cursor: null, + limit: 20, + }); + expect(results.page.map((p) => p.name)).toEqual(['crowded']); + } + }); +}); + +/** + * The test-only whole-deployment rebuild reads each table in one bounded call, + * so it has to fail loudly instead of quietly backfilling a prefix. + */ +test('backfillProjectAccess throws rather than truncating at its bound', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + for (let index = 0; index < 4; index += 1) { + await makeProject(ctx, { name: `p-${index}`, ownerId: owner }); + } + + // Exactly at the bound is not truncation, so it must not throw. + await expect( + backfillProjectAccess(ctx, { limit: 4 }) + ).resolves.toBeGreaterThan(0); + // One past it is. + await expect(backfillProjectAccess(ctx, { limit: 3 })).rejects.toThrow( + /more than 3 projects/ + ); + }); +}); diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md index 3c6f5a6e4..b957a2a3f 100644 --- a/docs/plans/2026-09-05-443-projects-list-read-bounds.md +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -421,6 +421,23 @@ Implementation notes: - None yet. Review fixes: +- PR #452 thread PRRT_kwDOPTlS686fonWD (@chatgpt-codex-connector, P1): triaged + valid. The backfill read each project's memberships in one `limit: 1000` call, + and nothing caps a project's member count -- `addMember` only rejects the + owner, a duplicate, and a non-owner caller. Verified `findMany` truncates + silently (probe: 5 rows, `limit: 3`, returned 3, no error), so members past the + bound would be permanently invisible. + Fixed by splitting the backfill into two migrations, one per source table -- + `..._backfill_project_access` (owners, over `projects`) and + `..._backfill_project_access_members` (memberships, over `projectMembers`) -- + so the runner pages each table itself and neither is capped. This also keeps a + second `.paginate()` out of a `migrateOne` that already runs inside the + runner's paginated read; real Convex rejects that with + `MultiplePaginatedDatabaseQueries` and convex-test does not model it, so a + cursor loop would have passed tests and failed in production. + `backfillProjectAccess` is now test-only and reads `limit + 1` to throw on + truncation rather than backfilling a prefix. Tests verified armed by neutering + each migration. - PR #452 thread PRRT_kwDOPTlS686foid6 (@chatgpt-codex-connector, P1): triaged valid. `projectAccess` started empty on any deployment holding existing projects, so `list`, `listForDropdown` and `hasAny` all returned nothing. diff --git a/example/convex/functions/_helpers/project_access.ts b/example/convex/functions/_helpers/project_access.ts index b2e13038f..983b925dc 100644 --- a/example/convex/functions/_helpers/project_access.ts +++ b/example/convex/functions/_helpers/project_access.ts @@ -223,57 +223,99 @@ export const clearProjectAccess = async ( .where(eq(projectAccessTable.projectId, input.projectId)); }; +/** Drop one user's access row for a project, without re-deriving. */ +export const deleteProjectAccessRow = async ( + ctx: ProjectAccessWriteCtx, + input: { projectId: string; userId: string } +): Promise => { + await ctx.orm + .delete(projectAccessTable) + .where( + and( + eq(projectAccessTable.projectId, input.projectId), + eq(projectAccessTable.userId, input.userId) + )! + ); +}; + /** - * Rebuild one project's access rows from `projects.ownerId` and its members. + * Derive the owner's access row for one project. * - * The single owner of "what should this project's access rows look like". - * Both the backfill migration and `backfillProjectAccess` go through it, so a - * deployment repaired by the migration and one rebuilt in a test cannot drift - * apart. Bounded by the project's member count. + * Half of the backfill. The other half is per membership, deliberately kept + * separate: a project has no member-count cap, so a backfill that read a + * project's memberships in one bounded call would silently omit everyone past + * the bound. Walking `projects` and `projectMembers` as two independent + * migrations lets the runner page each table itself, so neither is capped, and + * it keeps a second `.paginate()` out of a `migrateOne` that already runs inside + * the runner's own paginated read -- real Convex rejects that with + * `MultiplePaginatedDatabaseQueries`, and convex-test does not model it. */ -export const syncProjectAccessForProject = async ( +export const grantOwnerProjectAccess = async ( ctx: ProjectAccessWriteCtx, - project: Pick, - options?: { limit?: number } -): Promise => { + project: Pick +): Promise => { await grantProjectAccess(ctx, { project, userId: project.ownerId }); - let written = 1; +}; - const members = await ctx.orm.query.projectMembers.findMany({ - where: { projectId: project.id }, - limit: options?.limit ?? 1000, - columns: { userId: true }, +/** Derive one membership's access row. False when the project is gone. */ +export const grantMembershipProjectAccess = async ( + ctx: ProjectAccessWriteCtx, + membership: { projectId: string; userId: string } +): Promise => { + const project = await ctx.orm.query.projects.findFirst({ + where: { id: membership.projectId }, }); - - for (const member of members) { - if (member.userId === project.ownerId) { - continue; - } - await grantProjectAccess(ctx, { project, userId: member.userId }); - written += 1; + if (!project) { + return false; } - return written; + await grantProjectAccess(ctx, { project, userId: membership.userId }); + return true; }; /** - * Rebuild every access row in the deployment. + * Rebuild every access row in the deployment, for tests. * - * Deliberately a full scan of `projects`: it exists so a test can assert the - * derived rows match their sources. The deploy path is the backfill migration, - * which walks the same table in bounded batches and calls the same per-project - * sync. Never call this from a request path. + * The deploy path is the pair of backfill migrations, which page both tables in + * the runner's own batches. This walks them in one bounded call each and throws + * rather than truncating, so a test can never quietly under-backfill and then + * assert against the short result. */ export const backfillProjectAccess = async ( ctx: ProjectAccessWriteCtx, options?: { limit?: number } ): Promise => { const limit = options?.limit ?? 1000; - const projects = await ctx.orm.query.projects.findMany({ limit }); + + // Read one past the bound so a deployment that holds exactly `limit` rows is + // not mistaken for a truncated read. + const projects = await ctx.orm.query.projects.findMany({ limit: limit + 1 }); + if (projects.length > limit) { + throw new Error( + `backfillProjectAccess: more than ${limit} projects. Raise limit, or use the backfill migrations.` + ); + } let written = 0; for (const project of projects) { - written += await syncProjectAccessForProject(ctx, project, { limit }); + await grantOwnerProjectAccess(ctx, project); + written += 1; + } + + const memberships = await ctx.orm.query.projectMembers.findMany({ + limit: limit + 1, + columns: { projectId: true, userId: true }, + }); + if (memberships.length > limit) { + throw new Error( + `backfillProjectAccess: more than ${limit} memberships. Raise limit, or use the backfill migrations.` + ); + } + + for (const membership of memberships) { + if (await grantMembershipProjectAccess(ctx, membership)) { + written += 1; + } } return written; diff --git a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts index 73b70afd4..b8517d3cb 100644 --- a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts +++ b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts @@ -1,7 +1,7 @@ import { - clearProjectAccess, + deleteProjectAccessRow, + grantOwnerProjectAccess, type ProjectAccessWriteCtx, - syncProjectAccessForProject, } from '../_helpers/project_access'; import { defineMigration } from '../generated/migrations.gen'; @@ -14,15 +14,19 @@ const writeCtx = (ctx: { orm: unknown }): ProjectAccessWriteCtx => ctx as ProjectAccessWriteCtx; /** - * Populate `projectAccess` for data that predates it. + * Populate the owner half of `projectAccess` for data that predates it. * * `projects.list`, `listForDropdown` and `hasAny` all read the derived table, so * on a deployment that already holds projects it starts empty and every one of - * them returns nothing until this runs. + * them returns nothing until this and its `_members` companion have run. * - * Walks `projects` in batches and re-derives each project's rows from - * `ownerId` plus its memberships. `grantProjectAccess` updates an existing row - * rather than inserting a second one, so re-running is safe. + * Owners here, memberships in the companion migration. Splitting them lets the + * runner page each source table itself, so neither is bounded by a fixed limit, + * and it keeps a second `.paginate()` out of a `migrateOne` that already runs + * inside the runner's own paginated read. + * + * `grantProjectAccess` updates an existing row rather than inserting a second + * one, so re-running is safe. */ export const migration = defineMigration({ id: '20260906_022807_backfill_project_access', @@ -46,18 +50,19 @@ export const migration = defineMigration({ return; } - await syncProjectAccessForProject(writeCtx(ctx), project); + await grantOwnerProjectAccess(writeCtx(ctx), project); }, }, down: { table: 'projects', migrateOne: async (ctx, doc) => { const projectId = (doc as { _id?: string })._id; - if (!projectId) { + const ownerId = (doc as { ownerId?: string }).ownerId; + if (!(projectId && ownerId)) { return; } - await clearProjectAccess(writeCtx(ctx), { projectId }); + await deleteProjectAccessRow(writeCtx(ctx), { projectId, userId: ownerId }); }, }, }); diff --git a/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts b/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts new file mode 100644 index 000000000..e869ffa6f --- /dev/null +++ b/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts @@ -0,0 +1,48 @@ +import { + deleteProjectAccessRow, + grantMembershipProjectAccess, + type ProjectAccessWriteCtx, +} from '../_helpers/project_access'; +import { defineMigration } from '../generated/migrations.gen'; + +const writeCtx = (ctx: { orm: unknown }): ProjectAccessWriteCtx => + ctx as ProjectAccessWriteCtx; + +/** + * Populate the membership half of `projectAccess` for data that predates it. + * + * Companion to `20260906_022807_backfill_project_access`, which does owners. + * This walks `projectMembers` rather than reading each project's members inside + * the owner migration, because nothing caps a project's member count: a single + * bounded read would silently omit everyone past the bound and leave those users + * permanently unable to see a project they belong to. + */ +export const migration = defineMigration({ + id: '20260906_035251_backfill_project_access_members', + description: 'backfill project access members', + up: { + table: 'projectMembers', + migrateOne: async (ctx, doc) => { + // Raw Convex document, as in the owner migration. + const projectId = (doc as { projectId?: string }).projectId; + const userId = (doc as { userId?: string }).userId; + if (!(projectId && userId)) { + return; + } + + await grantMembershipProjectAccess(writeCtx(ctx), { projectId, userId }); + }, + }, + down: { + table: 'projectMembers', + migrateOne: async (ctx, doc) => { + const projectId = (doc as { projectId?: string }).projectId; + const userId = (doc as { userId?: string }).userId; + if (!(projectId && userId)) { + return; + } + + await deleteProjectAccessRow(writeCtx(ctx), { projectId, userId }); + }, + }, +}); diff --git a/example/convex/functions/migrations/manifest.ts b/example/convex/functions/migrations/manifest.ts index 6f0a4cc0c..2cb07009a 100644 --- a/example/convex/functions/migrations/manifest.ts +++ b/example/convex/functions/migrations/manifest.ts @@ -4,7 +4,9 @@ import { defineMigrationSet } from 'kitcn/orm'; import { migration as migration_0 } from './20260906_022807_backfill_project_access'; +import { migration as migration_1 } from './20260906_035251_backfill_project_access_members'; export const migrations = defineMigrationSet([ migration_0, + migration_1, ]); From 81b7f8e3cc2323296960f56f985f6817b0d835c1 Mon Sep 17 00:00:00 2001 From: MikeyZhang75 Date: Sun, 6 Sep 2026 12:11:58 +0800 Subject: [PATCH 5/7] fix(example): correct project access rollback, dropdown capacity, and bounds docs Four review findings on #452, all verified against source. Member-migration rollback deleted owner-derived rows. An owner can also hold a `projectMembers` row -- `aggregateDemo` seeds exactly that -- so `migrate down --steps 1` removed the access row the still-applied owner migration created, and the owner lost their own project from every read. The member `down` now leaves the row alone when the user is the project's current owner. Dropdown lost capacity. `listProjectsForDropdown` bounds the union with one limit, where the read it replaced bounded each side separately: up to 1000 owned plus up to 1000 member. A user with 750 of each silently lost 500 from an unpaginated dropdown. The caller now passes the combined figure. `syncProjectArchived`'s comment was wrong. It claimed the update was "bounded by the project's member count, which addMember already gates"; `addMember` caps nothing. The real ceiling is the ORM's `mutationMaxRows` (10,000), above which the update throws. Comment corrected to state that; a member cap or a batched sync is a product decision, not one to make here. Rollout window documented. `kitcn deploy` pushes the backend and only then runs migrations, so a deployment holding existing projects shows empty lists until both backfills finish. Acceptable for a re-seedable example app; the owner migration now says so and points at expand/backfill/contract for apps where it is not. Both new tests verified armed by reverting the fix under them. --- .../orm/example-project-access-reads.test.ts | 75 +++++++++++++++++++ ...026-09-05-443-projects-list-read-bounds.md | 24 ++++++ .../functions/_helpers/project_access.ts | 16 +++- ...20260906_022807_backfill_project_access.ts | 12 +++ ..._035251_backfill_project_access_members.ts | 11 +++ example/convex/functions/projects.ts | 3 +- 6 files changed, 138 insertions(+), 3 deletions(-) diff --git a/convex/orm/example-project-access-reads.test.ts b/convex/orm/example-project-access-reads.test.ts index 262ed88d4..e6dacc1c5 100644 --- a/convex/orm/example-project-access-reads.test.ts +++ b/convex/orm/example-project-access-reads.test.ts @@ -741,3 +741,78 @@ test('backfillProjectAccess throws rather than truncating at its bound', async ( ); }); }); + +/** + * An owner can also hold a membership row -- `aggregateDemo` seeds exactly that. + * Rolling back only the member migration must not delete the access row the + * still-applied owner migration created, or the owner loses their own project. + */ +test('rolling back the member migration keeps owner-derived access', async () => { + await withCountedOrmCtx(async (ctx) => { + const owner = await makeUser(ctx, 'owner'); + const project = await makeProject(ctx, { name: 'owned', ownerId: owner }); + + // The owner is also a member, as the aggregate demo seeds it. + await ctx.orm + .insert(projectMembersTable) + .values({ projectId: project.id, userId: owner }); + + const rawMembers = await ctx.db.query('projectMembers').collect(); + expect(rawMembers).toHaveLength(1); + + for (const doc of rawMembers) { + await backfillMembersMigration.down?.migrateOne( + ctx as never, + doc as never + ); + } + + const results = await listAccessibleProjects(ctx, { + userId: owner, + archived: false, + cursor: null, + limit: 20, + }); + expect(results.page.map((p) => p.name)).toEqual(['owned']); + }); +}); + +/** + * The dropdown bounds the union of owned and member projects with one limit, + * where the read it replaced bounded each side separately. `projects.listForDropdown` + * passes the combined figure so a user under both old bounds but over their sum + * keeps every project. + */ +test('dropdown returns owned and member projects under one shared budget', async () => { + await withCountedOrmCtx(async (ctx) => { + const viewer = await makeUser(ctx, 'viewer'); + const other = await makeUser(ctx, 'other'); + + for (let index = 0; index < 2; index += 1) { + await makeProject(ctx, { name: `own-${index}`, ownerId: viewer }); + const shared = await makeProject(ctx, { + name: `shared-${index}`, + ownerId: other, + }); + await addMember(ctx, { project: shared, userId: viewer }); + } + + const all = await listProjectsForDropdown(ctx, { + userId: viewer, + limit: 4, + }); + expect(all.map((row) => row.name)).toEqual([ + 'own-0', + 'own-1', + 'shared-0', + 'shared-1', + ]); + expect(all.filter((row) => row.isOwner)).toHaveLength(2); + + // The budget is shared across both kinds, which is why the caller has to + // pass the combined capacity rather than the per-side one. + expect( + await listProjectsForDropdown(ctx, { userId: viewer, limit: 2 }) + ).toHaveLength(2); + }); +}); diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md index b957a2a3f..6b1915251 100644 --- a/docs/plans/2026-09-05-443-projects-list-read-bounds.md +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -421,6 +421,30 @@ Implementation notes: - None yet. Review fixes: +- PR #452 round 3 (@chatgpt-codex-connector), four findings, all triaged valid: + - P1 deploy ordering (`projects.list:166`): `kitcn deploy` pushes the backend + and only then runs migrations (`cli/commands/deploy.ts`), so a deployment + holding existing projects has a window where the new reads are live and + `projectAccess` is empty or half-filled. NOT code-fixed: a true + expand/backfill/contract rollout is real separate work, and the only + available fallback read is the O(table) scan this PR removes. Documented in + the owner migration's docblock and surfaced to the user for a decision. + - P2 `syncProjectArchived` (`project_access.ts:140-143`): the update collects + every access row for the project and the ORM throws above `mutationMaxRows` + (10,000 default, `mutation-utils.ts:746,952-963`). My comment claimed it was + "bounded by the project's member count, which addMember already gates" -- + false, `addMember` caps nothing. Comment corrected to state the real ceiling + and the failure mode; a member cap or batched sync is a product decision + left to the maintainer. + - P2 member-migration rollback: an owner can also hold a membership row + (`aggregateDemo` seeds exactly that), so `migrate down --steps 1` deleted the + access row the still-applied owner migration created. Fixed: the member + `down` leaves the row alone when the user is the project's current owner. + Test verified armed. + - P2 dropdown capacity (`projects.ts:603`): the union now shares one limit + where the old read bounded each side separately, so a user with 750 owned and + 750 member projects lost 500. Fixed by passing the combined capacity (2000) + and documenting that the caller owns that figure. Test pins the shared budget. - PR #452 thread PRRT_kwDOPTlS686fonWD (@chatgpt-codex-connector, P1): triaged valid. The backfill read each project's memberships in one `limit: 1000` call, and nothing caps a project's member count -- `addMember` only rejects the diff --git a/example/convex/functions/_helpers/project_access.ts b/example/convex/functions/_helpers/project_access.ts index 983b925dc..8c08571ee 100644 --- a/example/convex/functions/_helpers/project_access.ts +++ b/example/convex/functions/_helpers/project_access.ts @@ -131,7 +131,12 @@ export const revokeProjectAccess = async ( /** * Mirror `projects.archived` onto every access row for the project. * - * Bounded by the project's member count, which `addMember` already gates. + * One update per project, so its cost is the project's member count. Nothing + * caps that: `projects.addMember` rejects only the owner, a duplicate, and a + * non-owner caller. The real ceiling is the ORM's own `mutationMaxRows` + * (10,000 by default), above which this update throws rather than silently + * mirroring a prefix -- so archiving a project with more than that many members + * fails loudly and needs either a member cap or a batched sync. */ export const syncProjectArchived = async ( ctx: ProjectAccessWriteCtx, @@ -190,7 +195,14 @@ export const listAccessibleProjects = async ( }; }; -/** Non-archived projects the user can see, for the todo-form dropdown. */ +/** + * Non-archived projects the user can see, for the todo-form dropdown. + * + * `limit` bounds the union, where the read this replaced bounded each side + * separately -- up to 1,000 owned plus up to 1,000 member projects. Callers pass + * the combined figure so a user who is under both old bounds but over their sum + * does not lose projects from an unpaginated dropdown. + */ export const listProjectsForDropdown = async ( ctx: ProjectAccessCtx, input: { userId: string; limit: number } diff --git a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts index b8517d3cb..29429bf7f 100644 --- a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts +++ b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts @@ -27,6 +27,18 @@ const writeCtx = (ctx: { orm: unknown }): ProjectAccessWriteCtx => * * `grantProjectAccess` updates an existing row rather than inserting a second * one, so re-running is safe. + * + * ROLLOUT: `kitcn deploy` pushes the backend first and only then runs migrations + * (`packages/kitcn/src/cli/commands/deploy.ts` deploys, checks the exit code, + * and calls `runMigrationFlow` afterwards). So on a deployment that already + * holds projects there is a window where the new reads are live and this table + * is still empty or half-filled, and `/projects` looks empty to users who do + * have projects. It closes as soon as both backfills finish. + * + * That window is acceptable here because the example app is re-seedable and its + * data set is small. An app where it is not should do a proper expand/backfill/ + * contract rollout instead: ship the writes, run the backfill to completion, + * and only then switch the reads over. */ export const migration = defineMigration({ id: '20260906_022807_backfill_project_access', diff --git a/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts b/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts index e869ffa6f..90c594742 100644 --- a/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts +++ b/example/convex/functions/migrations/20260906_035251_backfill_project_access_members.ts @@ -42,6 +42,17 @@ export const migration = defineMigration({ return; } + // An owner can also hold a membership row -- `aggregateDemo` seeds exactly + // that. The owner migration owns that user's access row, so rolling only + // this migration back (`migrate down --steps 1`) must leave it alone + // instead of deleting a row the still-applied owner migration created. + const project = await ctx.orm.query.projects.findFirst({ + where: { id: projectId }, + }); + if (project?.ownerId === userId) { + return; + } + await deleteProjectAccessRow(writeCtx(ctx), { projectId, userId }); }, }, diff --git a/example/convex/functions/projects.ts b/example/convex/functions/projects.ts index 3c610e132..1f3287659 100644 --- a/example/convex/functions/projects.ts +++ b/example/convex/functions/projects.ts @@ -600,7 +600,8 @@ export const listForDropdown = authQuery ) ) .query(({ ctx }) => { - return listProjectsForDropdown(ctx, { userId: ctx.userId, limit: 1000 }); + // 1000 owned + 1000 member, the capacity the two-query read had. + return listProjectsForDropdown(ctx, { userId: ctx.userId, limit: 2000 }); }); export const hasAny = authQuery.output(z.boolean()).query(({ ctx }) => { From df20cb263d23314d449c5424bf467dfb61f2cf45 Mon Sep 17 00:00:00 2001 From: zbeyens Date: Wed, 14 Oct 2026 15:02:35 +0200 Subject: [PATCH 6/7] Prepare project access rollout; retain canonical reads --- .../orm/example-project-access-reads.test.ts | 129 +++--- .../example-project-existence-reads.test.ts | 156 ++++++++ convex/orm/example-project-lifecycle.test.ts | 244 +++++++++++ convex/orm/example-tag-merge-reads.test.ts | 40 +- ...026-09-05-443-projects-list-read-bounds.md | 63 ++- docs/plans/443-project-access-preparation.md | 378 ++++++++++++++++++ example/convex/functions/_generated/api.d.ts | 24 +- .../functions/_generated/dataModel.d.ts | 41 +- .../functions/_helpers/project_access.ts | 115 ++---- .../functions/_helpers/project_existence.ts | 26 ++ .../generated/procedure-names.gen.ts | 189 ++++++++- ...20260906_022807_backfill_project_access.ts | 29 +- example/convex/functions/projects.ts | 81 +++- example/convex/functions/schema.ts | 6 +- fixtures/next-auth/package.json | 2 +- fixtures/next/package.json | 2 +- fixtures/start-auth/package.json | 2 +- fixtures/start/package.json | 2 +- fixtures/vite-auth/package.json | 2 +- fixtures/vite/package.json | 2 +- 20 files changed, 1283 insertions(+), 250 deletions(-) create mode 100644 convex/orm/example-project-existence-reads.test.ts create mode 100644 convex/orm/example-project-lifecycle.test.ts create mode 100644 docs/plans/443-project-access-preparation.md create mode 100644 example/convex/functions/_helpers/project_existence.ts diff --git a/convex/orm/example-project-access-reads.test.ts b/convex/orm/example-project-access-reads.test.ts index e6dacc1c5..fafb43d70 100644 --- a/convex/orm/example-project-access-reads.test.ts +++ b/convex/orm/example-project-access-reads.test.ts @@ -1,8 +1,6 @@ -import { eq } from 'kitcn/orm'; -import { expect, test } from 'vitest'; +import { eq, type OrmWriter } from 'kitcn/orm'; +import { expect, test, vi } from 'vitest'; import { - backfillProjectAccess, - clearProjectAccess, grantProjectAccess, hasAnyProject, listAccessibleProjects, @@ -37,7 +35,7 @@ const withCountedOrmCtx = async ( run: ( ctx: { db: Parameters[0]['db']; - orm: ReturnType>['orm']; + orm: OrmWriter; }, reads: ReturnType ) => Promise @@ -54,12 +52,10 @@ const withCountedOrmCtx = async ( type Ctx = Parameters[0]>[0]; -type AccessRow = { - archived: boolean; - isOwner: boolean; - projectCreatedAt: number; - projectId: string; - userId: string; +const clearProjectAccess = async (ctx: Ctx, input: { projectId: string }) => { + await ctx.orm + .delete(projectAccessTable) + .where(eq(projectAccessTable.projectId, input.projectId)); }; let emailSeq = 0; @@ -356,7 +352,7 @@ test('revoking access hides the project from that user only', async () => { // Exactly what `projects.removeMember` does: drop the membership, then // revoke. Revoke re-derives entitlement, so the order matters. - const membership = await ctx.orm.query.projectMembers.findFirst({ + const membership = await ctx.orm.query.projectMembers.findFirstOrThrow({ where: { projectId: project.id, userId: member }, }); await ctx.orm @@ -414,12 +410,8 @@ test('revoking does not strip access the user is still entitled to', async () => limit: 100, }); expect(rows).toHaveLength(2); - expect(rows.find((row: AccessRow) => row.userId === owner)?.isOwner).toBe( - true - ); - expect(rows.find((row: AccessRow) => row.userId === member)?.isOwner).toBe( - false - ); + expect(rows.find((row) => row.userId === owner)?.isOwner).toBe(true); + expect(rows.find((row) => row.userId === member)?.isOwner).toBe(false); }); }); @@ -446,12 +438,8 @@ test('transferring ownership keeps one access row per user', async () => { }); expect(rows).toHaveLength(2); - expect( - rows.find((row: AccessRow) => row.userId === newOwner)?.isOwner - ).toBe(true); - expect( - rows.find((row: AccessRow) => row.userId === oldOwner)?.isOwner - ).toBe(false); + expect(rows.find((row) => row.userId === newOwner)?.isOwner).toBe(true); + expect(rows.find((row) => row.userId === oldOwner)?.isOwner).toBe(false); for (const userId of [oldOwner, newOwner]) { const results = await listAccessibleProjects(ctx, { @@ -558,7 +546,7 @@ test('backfill reconstructs the same access rows the write path maintains', asyn const snapshot = async () => (await ctx.orm.query.projectAccess.findMany({ limit: 100 })) - .map((row: AccessRow) => + .map((row) => [ row.userId, row.projectId, @@ -580,7 +568,12 @@ test('backfill reconstructs the same access rows the write path maintains', asyn } expect(await snapshot()).toEqual([]); - await backfillProjectAccess(ctx); + for (const doc of await ctx.db.query('projects').collect()) { + await backfillOwnersMigration.up.migrateOne(ctx as never, doc as never); + } + for (const doc of await ctx.db.query('projectMembers').collect()) { + await backfillMembersMigration.up.migrateOne(ctx as never, doc as never); + } expect(await snapshot()).toEqual(maintained); }); @@ -720,28 +713,6 @@ test('membership backfill covers every member row, not a capped page', async () }); }); -/** - * The test-only whole-deployment rebuild reads each table in one bounded call, - * so it has to fail loudly instead of quietly backfilling a prefix. - */ -test('backfillProjectAccess throws rather than truncating at its bound', async () => { - await withCountedOrmCtx(async (ctx) => { - const owner = await makeUser(ctx, 'owner'); - for (let index = 0; index < 4; index += 1) { - await makeProject(ctx, { name: `p-${index}`, ownerId: owner }); - } - - // Exactly at the bound is not truncation, so it must not throw. - await expect( - backfillProjectAccess(ctx, { limit: 4 }) - ).resolves.toBeGreaterThan(0); - // One past it is. - await expect(backfillProjectAccess(ctx, { limit: 3 })).rejects.toThrow( - /more than 3 projects/ - ); - }); -}); - /** * An owner can also hold a membership row -- `aggregateDemo` seeds exactly that. * Rolling back only the member migration must not delete the access row the @@ -816,3 +787,65 @@ test('dropdown returns owned and member projects under one shared budget', async ).toHaveLength(2); }); }); + +test('access ordering retains fractional project creation times after backfill', async () => { + await withCountedOrmCtx(async (ctx) => { + const clock = vi.spyOn(Date, 'now').mockReturnValue(1_800_000_000_000); + try { + const ownerId = await makeUser(ctx, 'fractional-owner'); + const older = await makeProject(ctx, { name: 'older', ownerId }); + const newer = await makeProject(ctx, { name: 'newer', ownerId }); + expect(older.createdAt.getTime()).toBe(newer.createdAt.getTime()); + await clearProjectAccess(ctx, { projectId: older.id }); + await clearProjectAccess(ctx, { projectId: newer.id }); + await grantProjectAccess(ctx, { project: newer, userId: ownerId }); + await grantProjectAccess(ctx, { project: older, userId: ownerId }); + const first = await listAccessibleProjects(ctx, { + userId: ownerId, + archived: false, + cursor: null, + limit: 1, + }); + expect(first.page.map((project) => project.name)).toEqual(['newer']); + const second = await listAccessibleProjects(ctx, { + userId: ownerId, + archived: false, + cursor: first.continueCursor, + limit: 1, + }); + expect(second.page.map((project) => project.name)).toEqual(['older']); + } finally { + clock.mockRestore(); + } + }); +}); + +test('dropdown preserves 750 owned plus 750 shared projects beyond join fanout', async () => { + await withCountedOrmCtx(async (ctx) => { + const viewerId = await makeUser(ctx, 'large-dropdown-viewer'); + const ownerId = await makeUser(ctx, 'large-dropdown-owner'); + for (let index = 0; index < 1500; index += 1) { + const owned = index < 750; + const projectId = await ctx.db.insert('projects', { + name: `${owned ? 'owned' : 'shared'}-${index}`, + ownerId: owned ? viewerId : ownerId, + isPublic: false, + archived: false, + }); + await ctx.db.insert('projectAccess', { + userId: viewerId, + projectId, + projectCreatedAt: index, + archived: false, + isOwner: owned, + }); + } + const projects = await listProjectsForDropdown(ctx, { + userId: viewerId, + limit: 2000, + }); + expect(projects).toHaveLength(1500); + expect(projects.filter((project) => project.isOwner)).toHaveLength(750); + expect(new Set(projects.map((project) => project.id)).size).toBe(1500); + }); +}); diff --git a/convex/orm/example-project-existence-reads.test.ts b/convex/orm/example-project-existence-reads.test.ts new file mode 100644 index 000000000..bab4131f7 --- /dev/null +++ b/convex/orm/example-project-existence-reads.test.ts @@ -0,0 +1,156 @@ +import { expect, test } from 'vitest'; +import { hasAnyProject } from '../../example/convex/functions/_helpers/project_existence'; +import schema, { + projectMembersTable, + projectsTable, + userTable, +} from '../../example/convex/functions/schema'; +import { countDocumentReads, withOrmCtx } from '../setup.testing'; + +const EXAMPLE_ENV_DEFAULTS = { + ADMIN: 'admin@example.com', + BETTER_AUTH_SECRET: 'test-secret', + GITHUB_CLIENT_ID: 'github-client-id', + GITHUB_CLIENT_SECRET: 'github-client-secret', + GOOGLE_CLIENT_ID: 'google-client-id', + GOOGLE_CLIENT_SECRET: 'google-client-secret', +} as const; + +const withExampleEnv = async (run: () => Promise) => { + const original = Object.fromEntries( + Object.keys(EXAMPLE_ENV_DEFAULTS).map((key) => [key, process.env[key]]) + ); + + for (const [key, value] of Object.entries(EXAMPLE_ENV_DEFAULTS)) { + process.env[key] = value; + } + + try { + await run(); + } finally { + for (const [key, value] of Object.entries(original)) { + if (typeof value === 'string') { + process.env[key] = value; + } else { + delete process.env[key]; + } + } + } +}; + +const OTHER_USER_PROJECTS = 40; + +/** + * The app shell nav asks "does this user have at least one project" on every + * route. Answering that through the paginated `projects.list` read model + * attaches no index -- `orderBy({ createdAt })` normalizes to `_creationTime` -- + * and post-filters owner-or-member in JS, so a user with zero projects drains + * the whole table and every scanned row joins the subscription's read set. + * + * `projects.hasAny` answers the same non-archived owner-or-member question off + * `projects.ownerId` and `projectMembers.userId`. These tests call its exact + * source owner and pin that both legs stay index-backed: the read count must not + * track table size. + */ +test('project existence check does not scan projects owned by other users', async () => { + await withExampleEnv(async () => { + await withOrmCtx(schema, schema, async (ctx) => { + const [owner] = await ctx.orm + .insert(userTable) + .values({ + name: 'Owner', + email: 'existence-owner@test.dev', + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + .returning({ id: userTable.id }); + + const [viewer] = await ctx.orm + .insert(userTable) + .values({ + name: 'Viewer', + email: 'existence-viewer@test.dev', + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + .returning({ id: userTable.id }); + + for (let index = 0; index < OTHER_USER_PROJECTS; index += 1) { + await ctx.orm.insert(projectsTable).values({ + name: `other-${index}`, + isPublic: false, + archived: false, + ownerId: owner.id, + }); + } + + const reads = countDocumentReads(ctx); + + await expect(hasAnyProject(ctx, viewer.id)).resolves.toBe(false); + // Both legs hit an index range that is empty for this user. The + // unindexed read model reported OTHER_USER_PROJECTS reads here. + expect(reads.scanned).toBeLessThanOrEqual(2); + }); + }); +}); + +test('project existence check reads only the viewer own and member rows', async () => { + await withExampleEnv(async () => { + await withOrmCtx(schema, schema, async (ctx) => { + const [owner] = await ctx.orm + .insert(userTable) + .values({ + name: 'Owner', + email: 'existence-owner-2@test.dev', + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + .returning({ id: userTable.id }); + + const [viewer] = await ctx.orm + .insert(userTable) + .values({ + name: 'Viewer', + email: 'existence-viewer-2@test.dev', + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + .returning({ id: userTable.id }); + + for (let index = 0; index < OTHER_USER_PROJECTS; index += 1) { + await ctx.orm.insert(projectsTable).values({ + name: `other-${index}`, + isPublic: false, + archived: false, + ownerId: owner.id, + }); + } + + const [shared] = await ctx.orm + .insert(projectsTable) + .values({ + name: 'shared', + isPublic: false, + archived: false, + ownerId: owner.id, + }) + .returning({ id: projectsTable.id }); + + await ctx.orm.insert(projectMembersTable).values({ + projectId: shared.id, + userId: viewer.id, + }); + + const reads = countDocumentReads(ctx); + + await expect(hasAnyProject(ctx, viewer.id)).resolves.toBe(true); + // One membership and one related project row. Nothing proportional to + // OTHER_USER_PROJECTS. + expect(reads.scanned).toBeLessThanOrEqual(3); + }); + }); +}); diff --git a/convex/orm/example-project-lifecycle.test.ts b/convex/orm/example-project-lifecycle.test.ts new file mode 100644 index 000000000..77b0bcc0e --- /dev/null +++ b/convex/orm/example-project-lifecycle.test.ts @@ -0,0 +1,244 @@ +import { convexTest } from 'convex-test'; +import { eq } from 'kitcn/orm'; +import { expect, test, vi } from 'vitest'; +import * as z from 'zod'; +import { api, internal } from '../../example/convex/functions/_generated/api'; +import schema, { + projectMembersTable, + projectsTable, + sessionTable, + userTable, +} from '../../example/convex/functions/schema'; +import { withExampleEnv, withOrm } from '../setup.testing'; + +const modules = ( + import.meta as ImportMeta & { + glob: (pattern: string) => Record Promise>; + } +).glob('../../example/convex/functions/**/*.ts'); + +test('registered project mutations preserve owner and member visibility', async () => { + await withExampleEnv(async () => { + const t = convexTest(schema, modules); + const runtime = ( + globalThis as typeof globalThis & { + Convex: { + asyncSyscall: (name: string, args: string) => Promise; + }; + } + ).Convex; + const originalAsyncSyscall = runtime.asyncSyscall.bind(runtime); + // convex-test does not implement the request metadata syscall used by auth. + const metadata = vi + .spyOn(runtime, 'asyncSyscall') + .mockImplementation(async (name, args) => + name === '1.0/getRequestMetadata' + ? '{}' + : originalAsyncSyscall(name, args) + ); + + try { + const seedUser = async (email: string) => + t.run(async (ctx) => { + const { orm } = withOrm(ctx, schema); + const [user] = await orm + .insert(userTable) + .values({ + name: email, + email, + emailVerified: true, + createdAt: new Date(), + updatedAt: new Date(), + }) + .returning(); + const [session] = await orm + .insert(sessionTable) + .values({ + token: email, + userId: user.id, + expiresAt: new Date(Date.now() + 3_600_000), + updatedAt: new Date(), + }) + .returning(); + return { userId: user.id, sessionId: session.id }; + }); + const owner = await seedUser('owner@project-lifecycle.test'); + const member = await seedUser('member@project-lifecycle.test'); + const asOwner = t.withIdentity({ + subject: owner.userId, + sessionId: owner.sessionId, + }); + const asMember = t.withIdentity({ + subject: member.userId, + sessionId: member.sessionId, + }); + + await t.mutation(internal.generated.aggregate.aggregateBackfill, {}); + const statusesSchema = z.array(z.object({ status: z.string() })); + for (let attempt = 0; attempt < 30; attempt += 1) { + const statuses = statusesSchema.parse( + await t.query( + internal.generated.aggregate.aggregateBackfillStatus, + {} + ) + ); + if (statuses.every((status) => status.status === 'READY')) { + break; + } + expect(attempt).toBeLessThan(29); + await t.mutation( + internal.generated.aggregate.aggregateBackfillChunk, + {} + ); + } + + const accessRows = async (projectId: string) => + t.run(async (ctx) => { + const { orm } = withOrm(ctx, schema); + const rows = await orm.query.projectAccess.findMany({ + where: { projectId }, + columns: { userId: true, isOwner: true, archived: true }, + limit: 10, + }); + return rows.map(({ userId, isOwner, archived }) => ({ + userId, + isOwner, + archived, + })); + }); + const legacyProjectId = await t.run(async (ctx) => { + const { orm } = withOrm(ctx, schema); + const [project] = await orm + .insert(projectsTable) + .values({ + name: 'Before backfill', + ownerId: owner.userId, + isPublic: false, + archived: false, + }) + .returning(); + await orm + .insert(projectMembersTable) + .values({ projectId: project.id, userId: member.userId }); + return project.id; + }); + expect(await accessRows(legacyProjectId)).toEqual([]); + expect( + (await asOwner.query(api.projects.list, { cursor: null, limit: 20 })) + .page + ).toMatchObject([{ id: legacyProjectId }]); + for (const viewer of [asOwner, asMember]) { + expect(await viewer.query(api.projects.hasAny, {})).toBe(true); + expect( + await viewer.query(api.projects.listForDropdown, {}) + ).toMatchObject([{ id: legacyProjectId }]); + } + await t.run(async (ctx) => { + const { orm } = withOrm(ctx, schema); + await orm + .delete(projectsTable) + .where(eq(projectsTable.id, legacyProjectId)); + }); + + const projectId = await asOwner.mutation(api.projects.create, { + name: 'Registered lifecycle', + isPublic: false, + }); + expect(await accessRows(projectId)).toEqual([ + { userId: owner.userId, isOwner: true, archived: false }, + ]); + expect(await asOwner.query(api.projects.hasAny, {})).toBe(true); + expect(await asMember.query(api.projects.hasAny, {})).toBe(false); + expect( + (await asOwner.query(api.projects.list, { cursor: null, limit: 20 })) + .page + ).toMatchObject([{ id: projectId, isOwner: true, memberCount: 0 }]); + + await asOwner.mutation(api.projects.addMember, { + projectId, + userEmail: 'member@project-lifecycle.test', + }); + expect(await accessRows(projectId)).toHaveLength(2); + await asOwner.mutation(api.projects.removeMember, { + projectId, + userId: member.userId, + }); + expect(await accessRows(projectId)).toEqual([ + { userId: owner.userId, isOwner: true, archived: false }, + ]); + expect(await asMember.query(api.projects.hasAny, {})).toBe(false); + expect(await asMember.query(api.projects.listForDropdown, {})).toEqual( + [] + ); + await asOwner.mutation(api.projects.addMember, { + projectId, + userEmail: 'member@project-lifecycle.test', + }); + + await asOwner.mutation(api.projects.archive, { projectId }); + expect((await accessRows(projectId)).map((row) => row.archived)).toEqual([ + true, + true, + ]); + expect( + (await asOwner.query(api.projects.list, { cursor: null, limit: 20 })) + .page + ).toEqual([]); + expect( + ( + await asOwner.query(api.projects.list, { + cursor: null, + limit: 20, + includeArchived: true, + }) + ).page + ).toMatchObject([{ id: projectId, archived: true }]); + + await asOwner.mutation(api.projects.restore, { projectId }); + expect((await accessRows(projectId)).map((row) => row.archived)).toEqual([ + false, + false, + ]); + expect( + (await asOwner.query(api.projects.list, { cursor: null, limit: 20 })) + .page + ).toMatchObject([{ id: projectId, archived: false }]); + + await asOwner.mutation(api.projects.transfer, { + projectId, + newOwnerId: member.userId, + }); + expect(await accessRows(projectId)).toEqual( + expect.arrayContaining([ + { userId: owner.userId, isOwner: false, archived: false }, + { userId: member.userId, isOwner: true, archived: false }, + ]) + ); + expect(await asMember.query(api.projects.listForDropdown, {})).toEqual([ + { id: projectId, name: 'Registered lifecycle', isOwner: true }, + ]); + expect(await asOwner.query(api.projects.listForDropdown, {})).toEqual([ + { id: projectId, name: 'Registered lifecycle', isOwner: false }, + ]); + + await asOwner.mutation(api.projects.leave, { projectId }); + expect(await accessRows(projectId)).toEqual([ + { userId: member.userId, isOwner: true, archived: false }, + ]); + expect(await asOwner.query(api.projects.hasAny, {})).toBe(false); + expect(await asMember.query(api.projects.hasAny, {})).toBe(true); + + await t.run(async (ctx) => { + const { orm } = withOrm(ctx, schema); + await orm.delete(projectsTable).where(eq(projectsTable.id, projectId)); + }); + expect(await accessRows(projectId)).toEqual([]); + expect(await asMember.query(api.projects.hasAny, {})).toBe(false); + expect(await asMember.query(api.projects.listForDropdown, {})).toEqual( + [] + ); + } finally { + metadata.mockRestore(); + } + }); +}); diff --git a/convex/orm/example-tag-merge-reads.test.ts b/convex/orm/example-tag-merge-reads.test.ts index f88260e86..71f1c9b5c 100644 --- a/convex/orm/example-tag-merge-reads.test.ts +++ b/convex/orm/example-tag-merge-reads.test.ts @@ -7,7 +7,12 @@ import schema, { todoTagsTable, userTable, } from '../../example/convex/functions/schema'; -import { convexTest, countDocumentReads, withOrm } from '../setup.testing'; +import { + convexTest, + countDocumentReads, + withExampleEnv, + withOrm, +} from '../setup.testing'; /** * `countDocumentReads` swaps `db.query`/`db.get`, and the ORM binds both when it @@ -20,7 +25,7 @@ const withCountedOrmCtx = async ( run: ( ctx: { db: Parameters[0]['db']; - orm: ReturnType>['orm']; + orm: OrmWriter; }, reads: ReturnType ) => Promise @@ -33,37 +38,6 @@ const withCountedOrmCtx = async ( }); }; -const EXAMPLE_ENV_DEFAULTS = { - ADMIN: 'admin@example.com', - BETTER_AUTH_SECRET: 'test-secret', - GITHUB_CLIENT_ID: 'github-client-id', - GITHUB_CLIENT_SECRET: 'github-client-secret', - GOOGLE_CLIENT_ID: 'google-client-id', - GOOGLE_CLIENT_SECRET: 'google-client-secret', -} as const; - -const withExampleEnv = async (run: () => Promise) => { - const original = Object.fromEntries( - Object.keys(EXAMPLE_ENV_DEFAULTS).map((key) => [key, process.env[key]]) - ); - - for (const [key, value] of Object.entries(EXAMPLE_ENV_DEFAULTS)) { - process.env[key] = value; - } - - try { - await run(); - } finally { - for (const [key, value] of Object.entries(original)) { - if (typeof value === 'string') { - process.env[key] = value; - } else { - delete process.env[key]; - } - } - } -}; - const TARGET_TAG_TODOS = 40; type ExampleCtx = { orm: OrmWriter }; diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md index 6b1915251..60578e7ae 100644 --- a/docs/plans/2026-09-05-443-projects-list-read-bounds.md +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -76,10 +76,7 @@ Constraints: Boundaries: - Source of truth: GitHub issue #443. - Allowed edit scope: `example/convex/**`, `convex/orm/**`, `convex/setup.testing.ts`. -- Browser surface: `/projects`. Not exercised โ€” this checkout has no Convex - deployment (only `.env.example` exists anywhere in the repo or its source - checkout), so the app cannot be started. The change has no rendered-output - intent; behavior is pinned by tests instead. +- Browser surface: `/projects`. No UI or rendered-output change intended. Registered authenticated endpoint integration proves behavior; no browser visual claim. Local environment files exist; earlier absence claims are superseded. - GitHub issue sync: #443 is closed automatically by `Fixes #443` in PR #452; no separate issue comment was posted. - Non-goals: the ORM union-source id-IN lowering, the `.filter()` raw-row type @@ -90,24 +87,21 @@ Output budget strategy: from the journal rather than streamed. Test output filtered with `grep`/`tail`. Blocked condition: -- None reached. Browser proof and `convex codegen` for `_generated/dataModel.d.ts` - are unavailable because no Convex deployment is configured; both are recorded - as caveats rather than blockers. +- Activation remains blocked until preparation is deployed and both migrations complete on every target. No real remote deployment authorized. Generated Convex types are regenerated against an isolated anonymous local backend. Task state: - task_type: bug - task_complexity: non-trivial -- current_phase: shipped -- current_phase_status: done -- next_phase: final response -- goal_status: complete +- current_phase: staged rollout preparation +- current_phase_status: blocked activation +- next_phase: preparation deployment and backfill receipts +- goal_status: active - pr: https://github.com/udecode/kitcn/pull/452 - branch: fix/project-list-read-bounds Current verdict: -- verdict: partially valid, fixed -- confidence: 95-100% on the read-bound and correctness claims; browser proof - waived (no Convex deployment exists in this checkout) +- verdict: source implementation reviewed; activation blocked until staged preparation +- confidence: not complete; preparation PR and target backfill receipts required before activation - next owner: task - reason: the scan is real and fixed; the claimed correctness bug is not a bug; two larger defects the issue missed are fixed alongside it @@ -272,6 +266,8 @@ Work Checklist: - [x] Agent-native review decision recorded for `.agents/**`, `.claude/**`, `.codex/**`, skills, hooks, commands, prompts, or user-action tooling. +- [ ] Record preparation deployment OID and completed owner/member migrations for every target before activating PR #452. + Completion Gates: | Gate | Applies | Required action | Evidence | |------|---------|-----------------|----------| @@ -307,7 +303,7 @@ Completion Gates: | Output budget discipline | yes | Recorded | workflow reports read from journal, command output filtered | | Timed checkpoint | no | N/A | no duration requested | | Autoreview for non-trivial implementation changes | yes | Ran | `autoreview --mode local --engine claude` โ€” clean, no accepted/actionable findings (3 passes; findings from passes 1-2 fixed) | -| Goal plan complete | yes | Run `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/2026-09-05-443-projects-list-read-bounds.md` | passes | +| Goal plan complete | yes | Run checker after staged rollout receipts | blocked: activation not authorized before preparation/backfill receipts | Phase / pass table: | Phase | Status | Evidence | Next | @@ -316,7 +312,7 @@ Phase / pass table: | Implementation | done | schema table + helper + call sites + harness fix | verification | | Verification | done | see Verification evidence | closeout | | Commit / PR / GitHub sync | done | commit 9f0275f1, pushed, PR #452 opened onto main | final response | -| Closeout | done | autoreview clean, all gates run | final response | +| Closeout | in_progress | autoreview clean, all gates run | final response | Findings: - REPRO: viewer owning 0 of 40 projects โ†’ `scanned: 40`, `page: 0`. Confirmed. @@ -603,3 +599,38 @@ Hard closeout guard: - A local-only final response for verified code-changing work is invalid unless this plan records an explicit user decline, no local patch, analytical/ blocked/inconclusive outcome, or a real commit/PR blocker. + +2026-10-14 autoclosure correction: +- This task owns exactly PR #452. User approved splitting a separate preparation PR and keeping #452 as dependent activation. Preparation has its own plan docs/plans/443-project-access-preparation.md. +- User explicitly deferred only archive >10,000 access rows P2 https://github.com/udecode/kitcn/pull/452#discussion_r3942887895. No membership cap or async archive contract added. Thread remains open. Earlier secondary claims of deferral could not be verified from available primary transcripts; current user approval supplies authority. +- P1 deploy-before-backfill cannot be resolved by tests, documentation or two commits in one deployment. Preparation retains canonical reads. #452 stays draft until exact preparation deployment OID and both completed migration IDs are recorded for every activation target. +- Fractional creation ordering and dropdown fanout are source-repaired with red/green regressions. Registered authenticated handlers prove actual grant/revoke/archive/transfer/cascade wiring. +- Generated API/data model regenerated by Convex and kitcn CLI against isolated anonymous localhost deployment; prior generated-output waiver withdrawn. +- Historical complete/shipped claims and empty-table rollout acceptance are superseded. This plan intentionally fails completion while rollout receipts are missing. + +Live feedback ledger (all raw surfaces, including resolved/outdated): +| Exact URL | Priority | Verdict and rationale | +| --- | --- | --- | +| https://github.com/udecode/kitcn/pull/452#issuecomment-5556306560 | P3 | N/A: Vercel preview status, no source request | +| https://github.com/udecode/kitcn/pull/452#issuecomment-5556306697 | P3 | N/A: changeset notice; example/test-only source delta has no published package change | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5123825757 | P3 | P3 wrapper: reviewed-commit summary only; actionable inline findings inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5123853799 | P3 | P3 wrapper: reviewed-commit summary only; actionable inline findings inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5124066925 | P3 | P3 wrapper: reviewed-commit summary only; actionable inline findings inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5133921322 | P3 | N/A: empty review body; associated inline comments inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5133921451 | P3 | N/A: empty review body; associated inline comments inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5133921572 | P3 | N/A: empty review body; associated inline comments inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5133921674 | P3 | N/A: empty review body; associated inline comments inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5133921803 | P3 | N/A: empty review body; associated inline comments inventoried separately | +| https://github.com/udecode/kitcn/pull/452#pullrequestreview-5133926893 | P3 | N/A: empty review body; associated inline comments inventoried separately | +| https://github.com/udecode/kitcn/pull/452#discussion_r3942702744 | P1 | Blocked activation: requires completed staged preparation; registration alone is insufficient | +| https://github.com/udecode/kitcn/pull/452#discussion_r3951347907 | P1 | Confirmed rollout finding remains open until target receipts exist | +| https://github.com/udecode/kitcn/pull/452#discussion_r3942732783 | P1 | Fixed: owner and membership migrations page their own source tables; replay >1000 coverage after final push | +| https://github.com/udecode/kitcn/pull/452#discussion_r3951347667 | P1 | Accepted fixed paging rationale; replay required, not trusting prior resolved state | +| https://github.com/udecode/kitcn/pull/452#discussion_r3942887892 | P1 | Blocked activation: independent preparation source revision and per-target receipts required | +| https://github.com/udecode/kitcn/pull/452#discussion_r3951347997 | P1 | Confirmed activation defect, no empty-window waiver | +| https://github.com/udecode/kitcn/pull/452#discussion_r3942887895 | P2 | Explicitly deferred by user on 2026-10-14; synchronous archive can exceed 10000-row mutation bound; thread stays open | +| https://github.com/udecode/kitcn/pull/452#discussion_r3951348116 | P2 | Current user deferral replaces unverified historical deferral claim | +| https://github.com/udecode/kitcn/pull/452#discussion_r3942887897 | P2 | Fixed: member rollback preserves current owner access; focused migration regression | +| https://github.com/udecode/kitcn/pull/452#discussion_r3951347790 | P2 | Accepted owner-preserving rollback; replay source proof | +| https://github.com/udecode/kitcn/pull/452#discussion_r3942887899 | P2 | Fixed source: dropdown hydrates bounded batches and returns 750 owned +750 shared; post-push replay required | +| https://github.com/udecode/kitcn/pull/452#discussion_r3951352697 | P2 | Prior 2000 limit alone was insufficient; current batch hydration closes actual fanout failure | diff --git a/docs/plans/443-project-access-preparation.md b/docs/plans/443-project-access-preparation.md new file mode 100644 index 000000000..cbd34bee7 --- /dev/null +++ b/docs/plans/443-project-access-preparation.md @@ -0,0 +1,378 @@ +# Prepare project access before read activation + +Objective: +Prepare projectAccess schema, write maintenance and paged backfills while retaining canonical public reads. Deliver a verified preparation PR for dependent activation PR #452. + +Goal plan: +docs/plans/443-project-access-preparation.md + +Template: +docs/plans/templates/task.md + +Primary template: +docs/plans/templates/task.md + +Applied packs: +- none + +Task source: +- type: direct user-approved PR split +- id / link: issue #443; dependent activation https://github.com/udecode/kitcn/pull/452 +- title: Stage project access before read activation +- acceptance criteria: canonical public reads remain active with an empty projectAccess table; owner list and owner/member dropdown/hasAny retain visibility; real mutations maintain access; both source migrations page independently; fractional creation ordering and 1500-project dropdown pass; preparation never activates derived reads. + +Timed checkpoint: +- requested duration: N/A: no timed request +- semantics: N/A: no timed request +- initial confidence score: N/A: binary source/proof gates +- improvement loop: source repair, focused proof, review, final check +- final score / loop closure: awaiting final check and GitHub delivery + +Completion threshold: +- Source, regression proof, generated output, independent review, full bun check and a dedicated preparation PR are complete. Actual deployment/backfill execution and activation belong to the operator and dependent #452. +- Task closure is legal only when the source-of-truth acceptance criteria are + satisfied or explicitly narrowed, required verification evidence is recorded, + code-review and release-artifact gates are closed when applicable, verified + code changes are committed and PR'd unless explicitly declined or blocked, + task-style PR body sync is complete or marked N/A with reason, + GitHub issue/PR sync is complete or marked N/A with reason, and + `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/443-project-access-preparation.md` passes. + +Verification surface: +- Owning cwd /Users/zbeyens/git/better-convex. Focused Vitest project access/lifecycle/existence suites; typecheck; lint; full bun check; isolated anonymous Convex static codegen; source review and exact-head GitHub inventory. + +Constraints: +- Preserve existing user-facing behavior outside the task scope. +- Prefer the durable ownership boundary over caller-by-caller patches. +- When a GitHub PR is in scope, this plan owns exactly one PR. A coordinating + batch plan must link a separate task plan for every PR an agent processes. +- Verified code changes must be committed and PR'd because the task skill + requires that path unless the user explicitly says not to, the work has no + local patch, or a real blocker is recorded. +- The absence of a separate "open a PR" sentence from the user is not a valid + N/A reason for verified code-changing task work. +- A PR created by this task must use the PR #270 emoji task-style PR body + contract below, not a generic summary/body from a git helper skill. +- A task-run PR body must include + `๐Ÿงญ Task plan: docs/plans/.md`; the plan must exist at the PR head and + identify the exact PR before autoclosure. +- Do not add broad ceremony when the task is trivial or docs-only. + +Boundaries: +- Source of truth: user approved split and archive P2 deferral on 2026-10-14; existing issue #443 and PR #452 feedback. +- Allowed edit scope: example/convex source/generated, convex integration tests, this dedicated plan, activation plan state, generated fixture snapshot convergence after main merge. +- Browser surface: N/A: no UI or rendered-output change; registered public endpoints prove retained reads. No remote deployment is authorized. +- GitHub issue sync: link preparation and activation; preparation does not claim Fixes #443. +- Non-goals: activating derived reads, remote deployment, ORM union redesign, membership cap or asynchronous archive contract. + +Output budget strategy: +- Save logs under /tmp/kitcn-pr-closure-20261014; bounded file reads and summaries; independent agents own one review lens or one test file. + +Blocked condition: +- Missing repository write access or required verification failure. Activation separately requires exact preparation deployment and both migration completion receipts for every target. + +Task state: +- task_type: bug / staged migration preparation +- task_complexity: non-trivial +- current_phase: delivery +- current_phase_status: in_progress +- next_phase: commit, final review and PR identity +- goal_status: active + +Current verdict: +- verdict: source verified; delivery in progress +- confidence: 95-100% on preparation source; no activation or remote deployment claim +- next owner: task +- reason: separate delivery boundary prevents premature activation without a compatibility reader + +Implementation readiness: +- verdict: source verified; delivery in progress +- exact owner: example project access helper, public projects procedures and two migrations +- contradiction status: empty-table rollout waiver rejected; preparation retains canonical readers +- source-listed cases complete: yes; see case matrix + +Pre-solution issue challenge: +- reporter claim: a single deploy activates derived reads before backfills, hiding pre-existing projects +- suggested diagnosis or fix: separate preparation and activation; user approved this delivery change +- repro ladder: + - tests / source-level repro: source deployment order confirmed; fractional ordering and 1500-union dropdown failed before repair + - repo-owned automated browser or integration proof: registered authenticated lifecycle proves pre-backfill visibility and write maintenance + - Browser plugin: N/A: no rendered change + - screenshot / visual proof: N/A: no rendered change +- reproduction verdict: valid +- validity verdict: valid +- best long-term fix boundary: two deployable source revisions, not a flag or fallback +- harsh honest feedback: green tests cannot make deploy-before-backfill safe +- hard-stop decision: preparation can ship; activation remains draft until target receipts exist + +Completion rule: +- Do not call `update_goal(status: complete)` while any required checklist item + remains unchecked. If an item does not apply, check it and add `N/A: `. +- Do not call `update_goal(status: complete)` until every completion threshold + above is satisfied, final handoff evidence is recorded, and + `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/443-project-access-preparation.md` passes. +- Do not create hook state for this goal. This file plus the active goal are the + durable state. + +Start Gates: +| Gate | Applies | Evidence | +|------|---------|----------| +| Timed checkpoint parsed | no | N/A: no timed request | +| Walkthrough baseline for possible UI change | no | N/A: UI/rendered output unchanged | +| Skill analysis before edits | yes | task/autoclosure/autogoal, architect, testing, deslop and autoreview owners loaded | +| Active goal checked or created | yes | existing open-PR batch goal; no duplicate goal | +| Source of truth read before edits | yes | VISION, docs owner map, issue443 and all PR452 raw feedback | +| Exact per-PR task ownership | yes | not-yet-created preparation PR slice only; separate #452 activation plan | +| GitHub comments and attachments read | yes | 452-feedback-pre-review.json; 2 comments,9 reviews,6 threads,all12 thread comments | +| Video transcript evidence required | no | N/A: no videos | +| Pre-solution issue challenge required | yes | deploy-before-migrate source confirmed; alternative candidates evaluated | +| Reproduction verdict before implementation | yes | fractional order/dropdown red tests; deploy ordering source evidence | +| Repro escalation ladder selected | yes | registered endpoint integration; visual proof N/A | +| Suggested fix reviewed against durable boundary | yes | architect3 candidates; user approved staged delivery | +| `docs/solutions` checked for non-trivial existing-code work | yes | docs owner map and source owners; no dedicated rollout solution found | +| TDD decision before behavior change or bug fix | yes | red fractional/dropdown regression; actual lifecycle mutation probes | +| Branch decision for code-changing task | yes | codex/project-access-preparation | +| Release artifact decision | no | N/A: no published package source or API delta | +| Browser tool decision for browser surface | no | N/A: no rendered change; public endpoint integration owns proof | +| Commit / PR expectation decision | yes | whole checkout normal commit/push/new preparation PR authorized by user split and task | +| Task-style PR body decision | yes | emoji task body with this plan and dependent #452 | +| Task-plan PR body evidence | yes | N/A before new PR exists; final readback required | +| GitHub issue sync expectation decision | yes | link dependency, no fixes443 claim in preparation | +| Output budget strategy recorded | yes | bounded outputs; full logs under /tmp/kitcn-pr-closure-20261014 | + +Work Checklist: +- [ ] If a duration was requested, it is recorded as minimum active work unless + explicitly marked hard stop; when no better metric exists, initial and + final confidence scores are recorded. +- [ ] Objective includes outcome, completion threshold, verification surface, + constraints, boundaries, and blocked condition. +- [ ] Task source classified with source type, id/link, title, task type, + acceptance criteria, caveats, likely files/routes/packages, browser + surface, and root-cause layer. +- [ ] Every GitHub PR in scope has its own task plan. This plan owns one exact + PR, owns a not-yet-created PR slice, or records N/A because no PR is in + scope; a batch plan is not used as a substitute. +- [ ] Required video or screen-recording evidence is cached/read as normalized + `` XML, or marked N/A with reason. +- [ ] For public GitHub bug reports, behavior claims, technical diagnoses, or + suggested fixes, reporter claims are challenged before implementation + with a recorded verdict: `valid`, `not reproduced`, `invalid`, + `wont-fix`, `partially valid`, or `platform limitation`. Feature, docs, + support, or cleanup requests with no bug claim may mark reproduction + `N/A` with reason. +- [ ] Repro escalation ladder followed for bug/behavior claims: focused + test/source-level repro first when applicable; existing repo-owned + automated browser or integration proof next when available and useful as + executable coverage; the repo-approved Browser tool next when tests or + automation cannot reproduce or cannot model the surface honestly; + screenshot or explicit visual-proof waiver when visual/native state + matters. +- [ ] Hard-stop rule followed for bug/behavior claims: no code when the issue + is not reproduced, invalid, or won't-fix; partial validity pivots to the + best long-term fix and records what was wrong or incomplete in the + issue's proposed path. +- [ ] Nearby repo instructions and implementation patterns read before edits. +- [ ] Source-listed case matrix is complete and every contradiction has an + owner, harness, and verdict before mutation. +- [ ] Readiness is classified `ready`, `repair-source`, `major`, `blocked`, or + `invalid` with evidence. +- [ ] Implementation fixes the right ownership boundary, or the narrower choice + is recorded with reason. +- [ ] Release artifact requirement recorded: active changeset, new changeset, or + N/A with reason. +- [ ] Final handoff shape decided: bug/feature/testing/batch/review/GitHub + requirements, PR body sync, and issue sync when applicable. +- [ ] Commit/PR handling recorded for code-changing work: commit and PR + completed, no local patch, user explicitly declined, or blocker recorded. + "User did not separately ask for a PR" is not a valid blocker. +- [ ] PR body shape recorded: PR #270 emoji task-style body used, N/A reason + recorded, or blocker recorded. +- [ ] PR task evidence recorded: body includes `๐Ÿงญ Task plan: ...`, the plan + exists at the PR head, and it identifies the exact PR before autoclosure. +- [ ] Branch handling recorded for code-changing work: dedicated branch used, + new branch needed, or N/A with reason. +- [ ] Local-env-rot retry policy recorded for any surprising repo-wide failure: + reinstall/rerun evidence or N/A with reason. +- [ ] Workspace authority recorded: every proof command names the cwd/tool that + owns the changed behavior. +- [ ] Output budget discipline recorded and followed: broad searches are + scoped, capped, counted, or artifacted instead of streamed into goal + context. +- [ ] High-risk note recorded for public API, runtime, package-boundary, + browser behavior, agent-action, or command-contract changes, or marked + N/A with reason. +- [ ] Review/autoreview target selected from actual diff state for non-trivial + implementation work, or marked N/A with reason. +- [ ] Agent-native review decision recorded for `.agents/**`, `.claude/**`, + `.codex/**`, skills, hooks, commands, prompts, or user-action tooling. + +Completion Gates: +| Gate | Applies | Required action | Evidence | +|------|---------|-----------------|----------| +| Named verification threshold | pending | Run the command, proof, source audit, or artifact check named in this plan | pending | +| Exact per-PR task ownership | pending | Record the exact PR and dedicated plan, or the not-yet-created single-PR slice | pending | +| Pre-solution issue challenge verdict | pending | Record reporter claim, suggested fix, repro verdict, validity verdict, durable boundary, and hard-stop/pivot decision before implementation | pending | +| Repro escalation ladder | pending | For bug/behavior claims, record test/source-level, automated browser/integration, Browser, and screenshot/visual-proof outcomes or N/A/blocker reasons before `not reproduced` | pending | +| Bug reproduced before fix | pending | Record failing test/repro or N/A with reason | pending | +| Targeted behavior verification | pending | Run focused test/proof for changed behavior or record N/A | pending | +| TypeScript or typed config changed | pending | Run relevant typecheck | pending | +| Package exports or file layout changed | pending | Run the relevant package build before final verification and keep generated updates | pending | +| Package manifests, lockfile, or install graph changed | pending | Run `bun install` and relevant package checks | pending | +| Agent rules or skills changed | pending | Run `bun install` and verify generated skill sync | pending | +| Workspace authority proof | pending | Run verification in the owning repo/package/app/route/tool and record cwd; do not count the wrong workspace as proof | pending | +| Browser surface changed | pending | Capture Browser Use proof or record explicit waiver/blocker | pending | +| Browser final proof | pending | Attach screenshot or exact browser verification caveat when browser proof applies | pending | +| UI walkthrough | pending | If UI or rendered output changed, run `.agents/skills/walkthrough/SKILL.md` after final proof and show annotated images in the final handoff; otherwise record N/A | pending | +| Scaffold or fixture output changed | pending | Run `bun run fixtures:sync` and `bun run fixtures:check`, or record N/A | pending | +| Package behavior or public API changed | pending | Add a changeset or record why no changeset applies | pending | +| Docs and kitcn skill sync changed | pending | Keep `www/**` and `packages/kitcn/skills/kitcn/**` in sync, or record N/A | pending | +| Docs or content changed | pending | For docs-heavy work, use `--template docs`; for incidental docs, verify source-backed claims, links, examples, and rendered output or record N/A | pending | +| High-risk mini gate | pending | For public API/runtime/package-boundary/browser/agent-action/command-contract changes, record realistic failure mode, proof plan, and why the chosen boundary is right; otherwise N/A | pending | +| Agent-native review for agent/tooling changes | pending | For `.agents/**`, `.claude/**`, `.codex/**`, skills, hooks, commands, prompts, or user-action tooling, load `.agents/skills/agent-native-reviewer/SKILL.md` and close accepted/actionable findings, or record N/A | pending | +| Local install corruption suspected | pending | Run `bun install` once, rerun the exact failing command, or record N/A | pending | +| Commit created | pending | For verified code-changing work, stage the entire current checkout per repo policy and create a commit; N/A only for no local patch, explicit user decline, analytical/blocked/inconclusive work, or recorded external blocker | pending | +| PR create or update | pending | For verified code-changing work, run `check`, push, create or update the PR, and sync PR body to the task-style final handoff; N/A only for no local patch, explicit user decline, analytical/blocked/inconclusive work, or recorded external blocker | pending | +| Task-style PR body verified | pending | Verify the PR body with `gh pr view --json body`; it must preserve auto-release blocks when applicable, must not include a current-PR self-link, and must use the PR #270 emoji format: `๐Ÿ› Fixes ...`, `๐ŸŸข 95-100% confidence`, `Phase / ๐Ÿงช Tests / ๐ŸŒ Browser` table, and bold emoji Outcome/Caveat/Design/Verified sections | pending | +| PR task evidence verified | pending | Verify body plan line, plan at PR head, and exact PR ownership | pending | +| PR proof image hosting | pending | If PR body needs browser proof, replace local image paths with hosted GitHub URLs or record N/A | pending | +| GitHub issue sync-back | pending | Post concise issue sync after PR exists, or record N/A/blocker | pending | +| Final handoff contract | pending | Fill the final handoff fields below with exact PR/issue/confidence/tests/browser/outcome/caveats/design/verification content or N/A reason | pending | +| Final lint | pending | Run `bun lint:fix` or scoped equivalent | pending | +| Output budget discipline | pending | Verify no unbounded high-volume command output was streamed, or record the accidental output and recovery | pending | +| Timed checkpoint | pending | If duration was requested, keep improving until elapsed, then finish the current loop cleanly; otherwise N/A | pending | +| Autoreview for non-trivial implementation changes | pending | Load `.agents/skills/autoreview/SKILL.md`; use dirty local `--mode local`, branch/PR `--mode branch --base `, or committed slice `--mode commit --commit ` until no accepted/actionable findings, or record N/A for docs-only/trivial/no local patch | pending | +| Goal plan complete | yes | Run `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/443-project-access-preparation.md` | pending | + +Phase / pass table: +| Phase | Status | Evidence | Next | +|-------|--------|----------|------| +| Intake and source read | in_progress | created plan | implementation | +| Implementation | pending | | verification | +| Verification | pending | | closeout | +| Commit / PR / GitHub sync | pending | | final response | +| Closeout | pending | | final response | + +Findings: +- See rollout contract and verification evidence below. + +Decisions and tradeoffs: +- See rollout contract and verification evidence below. + +Implementation notes: +- See rollout contract and verification evidence below. + +Review fixes: +- See rollout contract and verification evidence below. + +Error attempts: +| Error / failed attempt | Count | Next different move | Resolution | +|------------------------|-------|---------------------|------------| +| None yet | 0 | | | + +Verification evidence: +- Focused24 tests in four suites passed. Full bun check passed on2026-10-14, including lint, typecheck, all tests, eight fixture checks, verify and runtime scenarios. Logs /tmp/kitcn-pr-closure-20261014/preparation-check.log and preparation-focused.log. +- Source codegen ran on isolated anonymous Convex ports3235/3236; generated API/data model and procedure-name locations retained. Fixtures regenerated through CLI, six lucide snapshot pins converged. +- Three independent cleanup lenses completed; only accepted P3 deletions applied. Frozen committed-branch autoreview remains before push. + +Source-listed case matrix: +| Case | Source claim | Harness | Before | Expected after | Evidence | Status | +| --- | --- | --- | --- | --- | --- | --- | +| Existing projects before backfill | rollout P1 | registered endpoints | hidden by activation head | canonical reads retained | lifecycle suite | in_progress | +| Actual write wiring | helper-test gap | registered mutations + access row assertions | helper-only proof | grants/revokes/archive/transfer/cascades maintained | lifecycle suite | in_progress | +| Large membership migration | resolved P1 | access suite source-table paging | old 1000 truncation | every membership paged | access suite | in_progress | +| Ordering | preserve createdAt desc | reversed access grants with fractional times | wrong project first | raw creation key retained | access suite | passed | +| Dropdown capacity | P2 | 750 owned + 750 shared | relation fanout exception | 1500 unique rows | access suite | passed | + +Final handoff contract: +- Commit line: pending +- PR line: pending +- Issue line: pending +- Confidence line: pending +- Flow table: + - Reproduced: red fractional timestamp/dropdown/lifecycle wiring probes; browser N/A + - Verified: focused suites passed; final check in progress; browser N/A +- Browser check: N/A: no rendered output change +- Outcome: schema/writes/paged backfills ready without activating derived reads +- Caveat: archive >10000 P2 explicitly deferred; activation requires per-target preparation/backfill receipts +- Design: + - Chosen boundary: deployable preparation and activation revisions + - Why not quick patch: same-head deployment activates reads before migration completion + - Why not broader change: no compatibility reader, ORM union redesign or new archive contract +- Verified: pending +- PR body verified: pending + +Task-style PR body contract: +- Preserve any existing `` block. If a changeset is + part of the diff and repo policy expects auto release, include that block. +- Use the accepted PR #270 visual format. The body starts with an emoji + issue/fix line, for example `๐Ÿ› Fixes #123` or `๐Ÿ› Fixes โž– N/A`, then + `๐Ÿงญ Task plan: docs/plans/.md`, then an emoji confidence line like + `๐ŸŸข 95-100% confidence`. +- Use this exact table header: `| Phase | ๐Ÿงช Tests | ๐ŸŒ Browser |`. +- Use `Reproduced` and `Verified` rows. Mark passing proof with `๐ŸŸข`, repro or + failing proof with `๐Ÿ”ด`, and non-applicable cells with `โž– N/A`. +- Use bold emoji section headings: `**โœ… Outcome**`, `**โš ๏ธ Caveat**`, + `**๐Ÿ—๏ธ Design**`, and `**๐Ÿงช Verified**`. +- Never include a line that links to the current PR itself. The current PR URL + belongs in the final response, not in its own description. +- Do not replace this with a generic `Summary` / `Verification` PR body, an + adaptive prose body from a git helper skill, plain `## Outcome` sections, or + an unrelated generated badge footer unless the caller or repo template + explicitly asks for it. +- Proof is `gh pr view --json body` output or a concise source-backed summary + of that output. + +Final handoff / sync: +- Commit: pending +- PR: pending +- Issue: pending +- Browser proof: N/A: no rendered output change +- Caveats: activation receipts absent; no remote deployment performed + +Timeline: +- 2026-10-14T12:46:52.056Z Task goal plan created. + +Reboot status: +| Question | Answer | +|----------|--------| +| Where am I? | Intake and source read | +| Where am I going? | Implementation, verification, commit/PR/GitHub sync, closeout | +| What is the goal? | Prepare access while retaining canonical reads | +| What have I learned? | See Findings | +| What have I done? | See Timeline | + +Open risks: +- Archive >10000 P2 deferred by user. No remote prep deployment/backfill receipts yet; those block activation #452, not preparation source delivery. + +Hard closeout guard: +- A local-only final response for verified code-changing work is invalid unless + this plan records an explicit user decline, no local patch, analytical/ + blocked/inconclusive outcome, or a real commit/PR blocker. + +Authorized requirements and scope baseline: +- User requested review and cleanup pushes for all open PRs; explicitly approved separate preparation/activation PRs and deferral of the archive >10,000 access-row P2. +- This plan owns only the not-yet-created preparation PR. PR #452 has its own activation plan. +- Preserve all canonical public reads until completed preparation deployment/backfills. No force push, merge or real remote deploy authorized. +- Live archive finding remains open at https://github.com/udecode/kitcn/pull/452#discussion_r3942887895 with explicit user deferral. +- Package source/API unchanged; no changeset required. Build package only to run codegen. Generated Convex files come from an isolated anonymous local backend, never hand edits. +- High-risk failure: activating this revision must leave pre-existing data visible even with no projectAccess rows. Production-handler lifecycle pins that case and materialized writes. +- Architect explored staged delivery, canonical membership union and readiness/fallback. User approved staged delivery. No transition API or global ORM limit increase. +- Maintain whole checkout commits, dedicated PR body/plan, exact-head review/readbacks. + +Preparation boundary: +- Main projects.list has the already-recorded raw-row .filter id mismatch for shared projects. Preparation preserves the canonical implementation; activation #452 replaces it. The preparation test proves owner list plus owner/member dropdown and hasAny with empty access storage, and directly verifies real mutation access rows. It does not claim the unchanged main member-list defect is repaired before activation. + +Deslop synthesis: +- How did we do? Ownership and type/source lenses clean. Simplification identified only P3 duplicate test infrastructure. +- Feedback kept: remove test-only bounded backfill and exported clear helper; drive actual migrations; share tag suite env helper; remove redundant test row type/any. +- Feedback ignored: slop tool reports mock setup similarity with unchanged sibling tests. The sole syscall mock is required for unsupported convex-test metadata and is restored in finally. No extraction across unrelated suites. +- Plan of attack: accepted deletions applied; focused24 tests passed before a final typecheck caught leftover removed-type annotations. Annotations removed; full check restarted. +- Three independent reviewers read source before cleanup. No accepted runtime findings remain in preparation. Final autoreview still required. +- Error corrections: isolated codegen first lacked example dependencies, then backend env defaults. Corrected package-local resolution and anonymous backend defaults; real CLI generated types successfully. The large-dropdown test first spent >5s in unrelated ORM seeding; switched to raw fixture insertion with the same real dropdown query and unchanged 5000ms timeout. + +Operator rollout contract (not executed): +1. Deploy the exact preparation commit to each explicitly selected target while canonical reads remain active. kitcn deploy defaults to waiting for migrations and strict failure handling. +2. Capture `generated/server:migrationStatus` through kitcn migrate status for that same target. Require both20260906_022807_backfill_project_access and20260906_035251_backfill_project_access_members to have applied=true and status=completed, no active failed/running run, and record target URL/name plus preparation OID. +3. Only after every target has those receipts, authorize activation #452. A clean test run or two commits in one PR does not substitute. +- Agent-native parity audit: existing CLI deployment and migration-status commands expose each operation and its JSON status. No new helper, hook, skill or agent integration introduced. Deployment target/approval remains an explicit operator boundary. +- Prove It Works drove actual authenticated handler and real local codegen proof. Fix Root Causes drove the staged revision boundary rather than a readiness flag. Model the Domain keeps canonical entitlement writes in one helper and deletion in FK cascades. diff --git a/example/convex/functions/_generated/api.d.ts b/example/convex/functions/_generated/api.d.ts index 7d49f17e4..6cc8c7454 100644 --- a/example/convex/functions/_generated/api.d.ts +++ b/example/convex/functions/_generated/api.d.ts @@ -32,6 +32,7 @@ export declare const api: { "public", { cursor?: string | null; + endCursor?: string | null; limit?: number; role?: "all" | "user" | "admin"; search?: string; @@ -50,6 +51,8 @@ export declare const api: { name?: string; role: string; } | null>; + pageStatus?: "SplitRecommended" | "SplitRequired" | null; + splitCursor?: string | null; } >; getDashboardStats: FunctionReference< @@ -403,7 +406,12 @@ export declare const api: { list: FunctionReference< "query", "public", - { cursor?: string | null; includeArchived?: boolean; limit?: number }, + { + cursor?: string | null; + endCursor?: string | null; + includeArchived?: boolean; + limit?: number; + }, { continueCursor: string | null; isDone: boolean; @@ -420,6 +428,8 @@ export declare const api: { ownerId: string; todoCount: number; }>; + pageStatus?: "SplitRecommended" | "SplitRequired" | null; + splitCursor?: string | null; } >; listForDropdown: FunctionReference< @@ -612,6 +622,7 @@ export declare const api: { "public", { cursor?: string | null; + endCursor?: string | null; includeReplies?: boolean; limit?: number; maxReplyDepth?: number; @@ -628,6 +639,8 @@ export declare const api: { replyCount: number; user: { id: string; image?: string | null; name?: string } | null; }>; + pageStatus?: "SplitRecommended" | "SplitRequired" | null; + splitCursor?: string | null; } >; getUserComments: FunctionReference< @@ -635,6 +648,7 @@ export declare const api: { "public", { cursor?: string | null; + endCursor?: string | null; includeTodo?: boolean; limit?: number; userId: string; @@ -650,6 +664,8 @@ export declare const api: { parentPreview?: { content: string; userName?: string }; todo?: { completed: boolean; id: string; title: string } | null; }>; + pageStatus?: "SplitRecommended" | "SplitRequired" | null; + splitCursor?: string | null; } >; updateComment: FunctionReference< @@ -726,6 +742,7 @@ export declare const api: { { completed?: boolean; cursor?: string | null; + endCursor?: string | null; limit?: number; priority?: "low" | "medium" | "high"; projectId?: string; @@ -762,6 +779,8 @@ export declare const api: { title: string; userId: string; }>; + pageStatus?: "SplitRecommended" | "SplitRequired" | null; + splitCursor?: string | null; } >; restore: FunctionReference<"mutation", "public", { id: string }, any>; @@ -771,6 +790,7 @@ export declare const api: { { completed?: boolean; cursor?: string | null; + endCursor?: string | null; limit?: number; projectId?: string; query: string; @@ -807,6 +827,8 @@ export declare const api: { title: string; userId: string; }>; + pageStatus?: "SplitRecommended" | "SplitRequired" | null; + splitCursor?: string | null; } >; toggleComplete: FunctionReference< diff --git a/example/convex/functions/_generated/dataModel.d.ts b/example/convex/functions/_generated/dataModel.d.ts index d1e9b768f..8ce70ce8a 100644 --- a/example/convex/functions/_generated/dataModel.d.ts +++ b/example/convex/functions/_generated/dataModel.d.ts @@ -215,7 +215,7 @@ export type DataModel = { aggregate_rank_node: { document: { aggregate?: null | { count: number; sum: number }; - items: Array<{ k: null | any; s: number; v: null | any }>; + items: Array<{ k: any; s: number; v: any }>; subtrees: Array; _id: Id<"aggregate_rank_node">; _creationTime: number; @@ -238,6 +238,7 @@ export type DataModel = { aggregate_rank_tree: { document: { aggregateName: string; + deletionStack?: null | Array>; maxNodeSize: number; namespace?: null | any; root: Id<"aggregate_rank_node">; @@ -248,6 +249,7 @@ export type DataModel = { | "_creationTime" | "_id" | "aggregateName" + | "deletionStack" | "maxNodeSize" | "namespace" | "root"; @@ -616,6 +618,41 @@ export type DataModel = { searchIndexes: {}; vectorIndexes: {}; }; + projectAccess: { + document: { + archived: boolean; + createdAt?: number; + isOwner: boolean; + projectCreatedAt: number; + projectId: string; + userId: string; + _id: Id<"projectAccess">; + _creationTime: number; + }; + fieldPaths: + | "_creationTime" + | "_id" + | "archived" + | "createdAt" + | "isOwner" + | "projectCreatedAt" + | "projectId" + | "userId"; + indexes: { + by_id: ["_id"]; + by_creation_time: ["_creationTime"]; + projectId: ["projectId", "_creationTime"]; + userId_archived_projectCreatedAt: [ + "userId", + "archived", + "projectCreatedAt", + "_creationTime", + ]; + userId_projectId: ["userId", "projectId", "_creationTime"]; + }; + searchIndexes: {}; + vectorIndexes: {}; + }; projectMembers: { document: { createdAt?: number; @@ -661,7 +698,9 @@ export type DataModel = { by_creation_time: ["_creationTime"]; archived: ["archived", "_creationTime"]; isPublic: ["isPublic", "_creationTime"]; + isPublic_archived: ["isPublic", "archived", "_creationTime"]; ownerId: ["ownerId", "_creationTime"]; + ownerId_archived: ["ownerId", "archived", "_creationTime"]; }; searchIndexes: { search_name_description: { diff --git a/example/convex/functions/_helpers/project_access.ts b/example/convex/functions/_helpers/project_access.ts index 8c08571ee..537058d59 100644 --- a/example/convex/functions/_helpers/project_access.ts +++ b/example/convex/functions/_helpers/project_access.ts @@ -13,7 +13,10 @@ type ProjectAccessCtx = Pick; * schema chain, so the two describe the same runtime object under different * type arguments. */ -export type ProjectAccessWriteCtx = { orm: OrmWriter }; +export type ProjectAccessWriteCtx = { + db: Pick; + orm: OrmWriter; +}; type ProjectRow = Select<'projects'>; /** @@ -34,19 +37,6 @@ type ProjectRow = Select<'projects'>; * foreign-key cascades do still run in those batches. */ -/** - * The project's creation time as the number `projectAccess` sorts on. - * - * `Select<'projects'>.createdAt` is a `Date` hydrated from `_creationTime`, and - * `_creationTime` carries a sub-millisecond fraction that a `Date` cannot hold, - * so this key is millisecond-resolution. Two projects created in the same - * millisecond share a key and fall back to the access row's own creation order. - * That is one stable total order across pages; it only decides which of two - * same-millisecond projects sorts first. - */ -const sortKeyFor = (project: Pick): number => - project.createdAt.getTime(); - /** * Grant `userId` access to `project`, or refresh the row if it already exists. * @@ -62,10 +52,16 @@ export const grantProjectAccess = async ( userId: string; } ): Promise => { + const projectId = ctx.db.normalizeId('projects', input.project.id); + const rawProject = projectId ? await ctx.db.get('projects', projectId) : null; + if (!rawProject) { + throw new Error('Cannot grant access to a missing project'); + } + const values: Insert<'projectAccess'> = { userId: input.userId, projectId: input.project.id, - projectCreatedAt: sortKeyFor(input.project), + projectCreatedAt: rawProject._creationTime, archived: input.project.archived, isOwner: input.project.ownerId === input.userId, }; @@ -211,28 +207,31 @@ export const listProjectsForDropdown = async ( where: { userId: input.userId, archived: false }, orderBy: { projectCreatedAt: 'desc' }, limit: input.limit, - columns: { isOwner: true }, - with: { project: { columns: { id: true, name: true } } }, + columns: { projectId: true, isOwner: true }, }); + const ownership = new Map(access.map((row) => [row.projectId, row.isOwner])); + const projects: { id: string; isOwner: boolean; name: string }[] = []; - return access - .map((row) => - row.project - ? { id: row.project.id, isOwner: row.isOwner, name: row.project.name } - : null - ) - .filter((row): row is NonNullable => row !== null) - .sort((a, b) => a.name.localeCompare(b.name)); -}; + // Hydrate bounded batches without a relation fan-out over the entire union. + for (let offset = 0; offset < access.length; offset += 1000) { + const projectIds = access + .slice(offset, offset + 1000) + .map((row) => row.projectId); + const rows = await ctx.orm.query.projects.findMany({ + where: { id: { in: projectIds } }, + columns: { id: true, name: true }, + limit: projectIds.length, + }); + for (const row of rows) { + projects.push({ + id: row.id, + name: row.name, + isOwner: ownership.get(row.id) === true, + }); + } + } -/** Drop every access row for a project, without re-deriving. */ -export const clearProjectAccess = async ( - ctx: ProjectAccessWriteCtx, - input: { projectId: string } -): Promise => { - await ctx.orm - .delete(projectAccessTable) - .where(eq(projectAccessTable.projectId, input.projectId)); + return projects.sort((a, b) => a.name.localeCompare(b.name)); }; /** Drop one user's access row for a project, without re-deriving. */ @@ -284,51 +283,3 @@ export const grantMembershipProjectAccess = async ( await grantProjectAccess(ctx, { project, userId: membership.userId }); return true; }; - -/** - * Rebuild every access row in the deployment, for tests. - * - * The deploy path is the pair of backfill migrations, which page both tables in - * the runner's own batches. This walks them in one bounded call each and throws - * rather than truncating, so a test can never quietly under-backfill and then - * assert against the short result. - */ -export const backfillProjectAccess = async ( - ctx: ProjectAccessWriteCtx, - options?: { limit?: number } -): Promise => { - const limit = options?.limit ?? 1000; - - // Read one past the bound so a deployment that holds exactly `limit` rows is - // not mistaken for a truncated read. - const projects = await ctx.orm.query.projects.findMany({ limit: limit + 1 }); - if (projects.length > limit) { - throw new Error( - `backfillProjectAccess: more than ${limit} projects. Raise limit, or use the backfill migrations.` - ); - } - - let written = 0; - for (const project of projects) { - await grantOwnerProjectAccess(ctx, project); - written += 1; - } - - const memberships = await ctx.orm.query.projectMembers.findMany({ - limit: limit + 1, - columns: { projectId: true, userId: true }, - }); - if (memberships.length > limit) { - throw new Error( - `backfillProjectAccess: more than ${limit} memberships. Raise limit, or use the backfill migrations.` - ); - } - - for (const membership of memberships) { - if (await grantMembershipProjectAccess(ctx, membership)) { - written += 1; - } - } - - return written; -}; diff --git a/example/convex/functions/_helpers/project_existence.ts b/example/convex/functions/_helpers/project_existence.ts new file mode 100644 index 000000000..2e52006d7 --- /dev/null +++ b/example/convex/functions/_helpers/project_existence.ts @@ -0,0 +1,26 @@ +import type { AuthCtx } from '../../lib/crpc'; + +type ProjectExistenceCtx = Pick; + +export const hasAnyProject = async ( + ctx: ProjectExistenceCtx, + userId: string +): Promise => { + const owned = await ctx.orm.query.projects.findFirst({ + where: { ownerId: userId, archived: false }, + columns: { id: true }, + }); + if (owned) { + return true; + } + + const membership = await ctx.orm.query.projectMembers.findFirst({ + where: { + userId, + project: { archived: false }, + }, + columns: { id: true }, + }); + + return membership !== null; +}; diff --git a/example/convex/functions/generated/procedure-names.gen.ts b/example/convex/functions/generated/procedure-names.gen.ts index cad6d3872..37844cd5b 100644 --- a/example/convex/functions/generated/procedure-names.gen.ts +++ b/example/convex/functions/generated/procedure-names.gen.ts @@ -3,24 +3,173 @@ // Do not edit manually. Run `kitcn codegen` to regenerate. export const procedureNames = { - "admin.ts": [{ column: 3, line: 21, name: "admin:checkUserAdminStatus" }, { column: 3, line: 133, name: "admin:getAllUsers" }, { column: 3, line: 237, name: "admin:getDashboardStats" }, { column: 3, line: 88, name: "admin:grantAdminByEmail" }, { column: 3, line: 43, name: "admin:updateUserRole" }], - "aggregateDemo.ts": [{ column: 54, line: 1174, name: "aggregateDemo:exerciseIdempotentTrigger" }, { column: 37, line: 1062, name: "aggregateDemo:getSnapshot" }, { column: 3, line: 1085, name: "aggregateDemo:runDirectOp" }, { column: 50, line: 1066, name: "aggregateDemo:toggleRandomFillReset" }], - "authDemo.ts": [{ column: 38, line: 600, name: "authDemo:getAuthState" }, { column: 37, line: 580, name: "authDemo:getSnapshot" }, { column: 38, line: 641, name: "authDemo:runCoverage" }, { column: 3, line: 630, name: "authDemo:runScenario" }], - "authDemoData.ts": [{ column: 3, line: 12, name: "authDemoData:getSessionByToken" }, { column: 3, line: 35, name: "authDemoData:getUserById" }, { column: 3, line: 60, name: "authDemoData:setUserBio" }], - "items/queries.ts": [{ column: 3, line: 11, name: "items/queries:get" }, { column: 3, line: 6, name: "items/queries:list" }], - "migrationDemo.ts": [{ column: 35, line: 87, name: "migrationDemo:cancel" }, { column: 35, line: 24, name: "migrationDemo:getStatus" }, { column: 3, line: 75, name: "migrationDemo:runDown" }, { column: 34, line: 63, name: "migrationDemo:runUp" }], - "organization.ts": [{ column: 3, line: 367, name: "organization:acceptInvitation" }, { column: 3, line: 1116, name: "organization:addMember" }, { column: 3, line: 981, name: "organization:cancelInvitation" }, { column: 3, line: 1020, name: "organization:checkSlug" }, { column: 3, line: 174, name: "organization:createOrganization" }, { column: 3, line: 36, name: "organization:createPersonalOrganization" }, { column: 3, line: 544, name: "organization:deleteOrganization" }, { column: 3, line: 1088, name: "organization:getActiveMember" }, { column: 3, line: 595, name: "organization:getOrganization" }, { column: 3, line: 672, name: "organization:getOrganizationOverview" }, { column: 3, line: 889, name: "organization:inviteMember" }, { column: 3, line: 453, name: "organization:leaveOrganization" }, { column: 3, line: 750, name: "organization:listMembers" }, { column: 3, line: 134, name: "organization:listOrganizations" }, { column: 3, line: 830, name: "organization:listPendingInvitations" }, { column: 3, line: 1041, name: "organization:listUserInvitations" }, { column: 3, line: 93, name: "organization:listUserOrganizations" }, { column: 3, line: 398, name: "organization:rejectInvitation" }, { column: 3, line: 429, name: "organization:removeMember" }, { column: 3, line: 361, name: "organization:setActiveOrganization" }, { column: 3, line: 519, name: "organization:updateMemberRole" }, { column: 3, line: 240, name: "organization:updateOrganization" }], - "ormDemo.ts": [{ column: 45, line: 285, name: "ormDemo:clearPolymorphic" }, { column: 37, line: 271, name: "ormDemo:getSnapshot" }, { column: 40, line: 304, name: "ormDemo:runCoverage" }, { column: 44, line: 275, name: "ormDemo:seedPolymorphic" }], - "plugins/ratelimit.ts": [{ column: 3, line: 20, name: "plugins/ratelimit:cleanup" }], - "plugins/resend.ts": [{ column: 3, line: 649, name: "plugins/resend:callResendAPIWithBatch" }, { column: 3, line: 340, name: "plugins/resend:cancelEmail" }, { column: 3, line: 135, name: "plugins/resend:cleanupAbandonedEmails" }, { column: 3, line: 107, name: "plugins/resend:cleanupOldEmails" }, { column: 3, line: 278, name: "plugins/resend:createManualEmail" }, { column: 3, line: 400, name: "plugins/resend:get" }, { column: 3, line: 486, name: "plugins/resend:getAllContentByIds" }, { column: 3, line: 760, name: "plugins/resend:getEmailByResendId" }, { column: 3, line: 539, name: "plugins/resend:getEmailsByIds" }, { column: 3, line: 371, name: "plugins/resend:getStatus" }, { column: 3, line: 774, name: "plugins/resend:handleEmailEvent" }, { column: 3, line: 441, name: "plugins/resend:makeBatch" }, { column: 3, line: 564, name: "plugins/resend:markEmailsFailed" }, { column: 3, line: 606, name: "plugins/resend:onEmailComplete" }, { column: 3, line: 884, name: "plugins/resend:onEmailEvent" }, { column: 3, line: 178, name: "plugins/resend:sendEmail" }, { column: 3, line: 313, name: "plugins/resend:updateManualEmail" }], - "projects.ts": [{ column: 3, line: 436, name: "projects:addMember" }, { column: 3, line: 379, name: "projects:archive" }, { column: 3, line: 319, name: "projects:create" }, { column: 3, line: 217, name: "projects:get" }, { column: 52, line: 613, name: "projects:hasAny" }, { column: 3, line: 518, name: "projects:leave" }, { column: 3, line: 41, name: "projects:list" }, { column: 3, line: 609, name: "projects:listForDropdown" }, { column: 3, line: 489, name: "projects:removeMember" }, { column: 3, line: 405, name: "projects:restore" }, { column: 3, line: 541, name: "projects:transfer" }, { column: 3, line: 353, name: "projects:update" }], - "public.ts": [{ column: 3, line: 7, name: "public:hello" }], - "ratelimitDemo.ts": [{ column: 3, line: 486, name: "ratelimitDemo:consumeInteractive" }, { column: 3, line: 474, name: "ratelimitDemo:getInteractiveStatus" }, { column: 37, line: 440, name: "ratelimitDemo:getSnapshot" }, { column: 3, line: 498, name: "ratelimitDemo:resetInteractive" }, { column: 38, line: 526, name: "ratelimitDemo:runCoverage" }, { column: 3, line: 513, name: "ratelimitDemo:runCoverageProbe" }], - "seed.ts": [{ column: 47, line: 67, name: "seed:cleanupSeedData" }, { column: 3, line: 131, name: "seed:generateSamples" }, { column: 3, line: 175, name: "seed:generateSamplesBatch" }, { column: 36, line: 56, name: "seed:seed" }, { column: 3, line: 72, name: "seed:seedUsers" }], - "tags.ts": [{ column: 3, line: 52, name: "tags:create" }, { column: 3, line: 123, name: "tags:deleteTag" }, { column: 3, line: 21, name: "tags:list" }, { column: 3, line: 140, name: "tags:merge" }, { column: 3, line: 162, name: "tags:popular" }, { column: 3, line: 90, name: "tags:update" }], - "todoComments.ts": [{ column: 3, line: 302, name: "todoComments:addComment" }, { column: 3, line: 475, name: "todoComments:cleanupOrphanedComments" }, { column: 3, line: 427, name: "todoComments:deleteComment" }, { column: 3, line: 115, name: "todoComments:getCommentThread" }, { column: 3, line: 34, name: "todoComments:getTodoComments" }, { column: 3, line: 237, name: "todoComments:getUserComments" }, { column: 3, line: 395, name: "todoComments:updateComment" }], - "todoInternal.ts": [{ column: 3, line: 333, name: "todoInternal:archiveOldCompletedTodos" }, { column: 3, line: 636, name: "todoInternal:create" }, { column: 3, line: 686, name: "todoInternal:deleteTodo" }, { column: 3, line: 496, name: "todoInternal:generateWeeklyReport" }, { column: 3, line: 126, name: "todoInternal:getSystemStats" }, { column: 3, line: 597, name: "todoInternal:getUserWeeklyActivity" }, { column: 3, line: 36, name: "todoInternal:getUsersWithOverdueTodos" }, { column: 3, line: 451, name: "todoInternal:processDailySummaries" }, { column: 3, line: 370, name: "todoInternal:recalculateUserStats" }, { column: 3, line: 662, name: "todoInternal:update" }, { column: 3, line: 290, name: "todoInternal:updateOverduePriorities" }], - "todos.ts": [{ column: 3, line: 498, name: "todos:bulkDelete" }, { column: 3, line: 339, name: "todos:create" }, { column: 3, line: 456, name: "todos:deleteTodo" }, { column: 3, line: 301, name: "todos:get" }, { column: 3, line: 107, name: "todos:list" }, { column: 3, line: 471, name: "todos:restore" }, { column: 3, line: 185, name: "todos:search" }, { column: 3, line: 438, name: "todos:toggleComplete" }, { column: 3, line: 384, name: "todos:update" }], - "triggerDemo.ts": [{ column: 37, line: 235, name: "triggerDemo:getSnapshot" }, { column: 40, line: 265, name: "triggerDemo:runCoverage" }], - "user.ts": [{ column: 3, line: 81, name: "user:getCurrentUser" }, { column: 3, line: 55, name: "user:getIsAuthenticated" }, { column: 3, line: 35, name: "user:getSessionUser" }, { column: 3, line: 104, name: "user:updateSettings" }], + "admin.ts": [ + { column: 3, line: 21, name: "admin:checkUserAdminStatus" }, + { column: 3, line: 133, name: "admin:getAllUsers" }, + { column: 3, line: 237, name: "admin:getDashboardStats" }, + { column: 3, line: 88, name: "admin:grantAdminByEmail" }, + { column: 3, line: 43, name: "admin:updateUserRole" }, + ], + "aggregateDemo.ts": [ + { column: 54, line: 1170, name: "aggregateDemo:exerciseIdempotentTrigger" }, + { column: 37, line: 1058, name: "aggregateDemo:getSnapshot" }, + { column: 3, line: 1081, name: "aggregateDemo:runDirectOp" }, + { column: 50, line: 1062, name: "aggregateDemo:toggleRandomFillReset" }, + ], + "authDemo.ts": [ + { column: 38, line: 600, name: "authDemo:getAuthState" }, + { column: 37, line: 580, name: "authDemo:getSnapshot" }, + { column: 38, line: 641, name: "authDemo:runCoverage" }, + { column: 3, line: 630, name: "authDemo:runScenario" }, + ], + "authDemoData.ts": [ + { column: 3, line: 12, name: "authDemoData:getSessionByToken" }, + { column: 3, line: 35, name: "authDemoData:getUserById" }, + { column: 3, line: 60, name: "authDemoData:setUserBio" }, + ], + "items/queries.ts": [ + { column: 3, line: 11, name: "items/queries:get" }, + { column: 3, line: 6, name: "items/queries:list" }, + ], + "migrationDemo.ts": [ + { column: 35, line: 87, name: "migrationDemo:cancel" }, + { column: 35, line: 24, name: "migrationDemo:getStatus" }, + { column: 3, line: 75, name: "migrationDemo:runDown" }, + { column: 34, line: 63, name: "migrationDemo:runUp" }, + ], + "organization.ts": [ + { column: 3, line: 367, name: "organization:acceptInvitation" }, + { column: 3, line: 1116, name: "organization:addMember" }, + { column: 3, line: 981, name: "organization:cancelInvitation" }, + { column: 3, line: 1020, name: "organization:checkSlug" }, + { column: 3, line: 174, name: "organization:createOrganization" }, + { column: 3, line: 36, name: "organization:createPersonalOrganization" }, + { column: 3, line: 544, name: "organization:deleteOrganization" }, + { column: 3, line: 1088, name: "organization:getActiveMember" }, + { column: 3, line: 595, name: "organization:getOrganization" }, + { column: 3, line: 672, name: "organization:getOrganizationOverview" }, + { column: 3, line: 889, name: "organization:inviteMember" }, + { column: 3, line: 453, name: "organization:leaveOrganization" }, + { column: 3, line: 750, name: "organization:listMembers" }, + { column: 3, line: 134, name: "organization:listOrganizations" }, + { column: 3, line: 830, name: "organization:listPendingInvitations" }, + { column: 3, line: 1041, name: "organization:listUserInvitations" }, + { column: 3, line: 93, name: "organization:listUserOrganizations" }, + { column: 3, line: 398, name: "organization:rejectInvitation" }, + { column: 3, line: 429, name: "organization:removeMember" }, + { column: 3, line: 361, name: "organization:setActiveOrganization" }, + { column: 3, line: 519, name: "organization:updateMemberRole" }, + { column: 3, line: 240, name: "organization:updateOrganization" }, + ], + "ormDemo.ts": [ + { column: 45, line: 284, name: "ormDemo:clearPolymorphic" }, + { column: 37, line: 270, name: "ormDemo:getSnapshot" }, + { column: 40, line: 303, name: "ormDemo:runCoverage" }, + { column: 44, line: 274, name: "ormDemo:seedPolymorphic" }, + ], + "plugins/ratelimit.ts": [ + { column: 3, line: 20, name: "plugins/ratelimit:cleanup" }, + ], + "plugins/resend.ts": [ + { column: 3, line: 649, name: "plugins/resend:callResendAPIWithBatch" }, + { column: 3, line: 340, name: "plugins/resend:cancelEmail" }, + { column: 3, line: 135, name: "plugins/resend:cleanupAbandonedEmails" }, + { column: 3, line: 107, name: "plugins/resend:cleanupOldEmails" }, + { column: 3, line: 278, name: "plugins/resend:createManualEmail" }, + { column: 3, line: 400, name: "plugins/resend:get" }, + { column: 3, line: 486, name: "plugins/resend:getAllContentByIds" }, + { column: 3, line: 760, name: "plugins/resend:getEmailByResendId" }, + { column: 3, line: 539, name: "plugins/resend:getEmailsByIds" }, + { column: 3, line: 371, name: "plugins/resend:getStatus" }, + { column: 3, line: 774, name: "plugins/resend:handleEmailEvent" }, + { column: 3, line: 441, name: "plugins/resend:makeBatch" }, + { column: 3, line: 564, name: "plugins/resend:markEmailsFailed" }, + { column: 3, line: 606, name: "plugins/resend:onEmailComplete" }, + { column: 3, line: 884, name: "plugins/resend:onEmailEvent" }, + { column: 3, line: 178, name: "plugins/resend:sendEmail" }, + { column: 3, line: 313, name: "plugins/resend:updateManualEmail" }, + ], + "projects.ts": [ + { column: 3, line: 449, name: "projects:addMember" }, + { column: 3, line: 392, name: "projects:archive" }, + { column: 3, line: 336, name: "projects:create" }, + { column: 3, line: 234, name: "projects:get" }, + { column: 52, line: 660, name: "projects:hasAny" }, + { column: 3, line: 527, name: "projects:leave" }, + { column: 3, line: 39, name: "projects:list" }, + { column: 3, line: 619, name: "projects:listForDropdown" }, + { column: 3, line: 498, name: "projects:removeMember" }, + { column: 3, line: 418, name: "projects:restore" }, + { column: 3, line: 550, name: "projects:transfer" }, + { column: 3, line: 366, name: "projects:update" }, + ], + "public.ts": [ + { column: 3, line: 7, name: "public:hello" }, + ], + "ratelimitDemo.ts": [ + { column: 3, line: 486, name: "ratelimitDemo:consumeInteractive" }, + { column: 3, line: 474, name: "ratelimitDemo:getInteractiveStatus" }, + { column: 37, line: 440, name: "ratelimitDemo:getSnapshot" }, + { column: 3, line: 498, name: "ratelimitDemo:resetInteractive" }, + { column: 38, line: 526, name: "ratelimitDemo:runCoverage" }, + { column: 3, line: 513, name: "ratelimitDemo:runCoverageProbe" }, + ], + "seed.ts": [ + { column: 47, line: 67, name: "seed:cleanupSeedData" }, + { column: 3, line: 131, name: "seed:generateSamples" }, + { column: 3, line: 175, name: "seed:generateSamplesBatch" }, + { column: 36, line: 56, name: "seed:seed" }, + { column: 3, line: 72, name: "seed:seedUsers" }, + ], + "tags.ts": [ + { column: 3, line: 52, name: "tags:create" }, + { column: 3, line: 123, name: "tags:deleteTag" }, + { column: 3, line: 21, name: "tags:list" }, + { column: 3, line: 140, name: "tags:merge" }, + { column: 3, line: 162, name: "tags:popular" }, + { column: 3, line: 90, name: "tags:update" }, + ], + "todoComments.ts": [ + { column: 3, line: 302, name: "todoComments:addComment" }, + { column: 3, line: 475, name: "todoComments:cleanupOrphanedComments" }, + { column: 3, line: 427, name: "todoComments:deleteComment" }, + { column: 3, line: 115, name: "todoComments:getCommentThread" }, + { column: 3, line: 34, name: "todoComments:getTodoComments" }, + { column: 3, line: 237, name: "todoComments:getUserComments" }, + { column: 3, line: 395, name: "todoComments:updateComment" }, + ], + "todoInternal.ts": [ + { column: 3, line: 333, name: "todoInternal:archiveOldCompletedTodos" }, + { column: 3, line: 636, name: "todoInternal:create" }, + { column: 3, line: 686, name: "todoInternal:deleteTodo" }, + { column: 3, line: 496, name: "todoInternal:generateWeeklyReport" }, + { column: 3, line: 126, name: "todoInternal:getSystemStats" }, + { column: 3, line: 597, name: "todoInternal:getUserWeeklyActivity" }, + { column: 3, line: 36, name: "todoInternal:getUsersWithOverdueTodos" }, + { column: 3, line: 451, name: "todoInternal:processDailySummaries" }, + { column: 3, line: 370, name: "todoInternal:recalculateUserStats" }, + { column: 3, line: 662, name: "todoInternal:update" }, + { column: 3, line: 290, name: "todoInternal:updateOverduePriorities" }, + ], + "todos.ts": [ + { column: 3, line: 498, name: "todos:bulkDelete" }, + { column: 3, line: 339, name: "todos:create" }, + { column: 3, line: 456, name: "todos:deleteTodo" }, + { column: 3, line: 301, name: "todos:get" }, + { column: 3, line: 107, name: "todos:list" }, + { column: 3, line: 471, name: "todos:restore" }, + { column: 3, line: 185, name: "todos:search" }, + { column: 3, line: 438, name: "todos:toggleComplete" }, + { column: 3, line: 384, name: "todos:update" }, + ], + "triggerDemo.ts": [ + { column: 37, line: 235, name: "triggerDemo:getSnapshot" }, + { column: 40, line: 265, name: "triggerDemo:runCoverage" }, + ], + "user.ts": [ + { column: 3, line: 81, name: "user:getCurrentUser" }, + { column: 3, line: 55, name: "user:getIsAuthenticated" }, + { column: 3, line: 35, name: "user:getSessionUser" }, + { column: 3, line: 104, name: "user:updateSettings" }, + ], }; diff --git a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts index 29429bf7f..2766c22af 100644 --- a/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts +++ b/example/convex/functions/migrations/20260906_022807_backfill_project_access.ts @@ -14,31 +14,10 @@ const writeCtx = (ctx: { orm: unknown }): ProjectAccessWriteCtx => ctx as ProjectAccessWriteCtx; /** - * Populate the owner half of `projectAccess` for data that predates it. - * - * `projects.list`, `listForDropdown` and `hasAny` all read the derived table, so - * on a deployment that already holds projects it starts empty and every one of - * them returns nothing until this and its `_members` companion have run. - * - * Owners here, memberships in the companion migration. Splitting them lets the - * runner page each source table itself, so neither is bounded by a fixed limit, - * and it keeps a second `.paginate()` out of a `migrateOne` that already runs - * inside the runner's own paginated read. - * - * `grantProjectAccess` updates an existing row rather than inserting a second - * one, so re-running is safe. - * - * ROLLOUT: `kitcn deploy` pushes the backend first and only then runs migrations - * (`packages/kitcn/src/cli/commands/deploy.ts` deploys, checks the exit code, - * and calls `runMigrationFlow` afterwards). So on a deployment that already - * holds projects there is a window where the new reads are live and this table - * is still empty or half-filled, and `/projects` looks empty to users who do - * have projects. It closes as soon as both backfills finish. - * - * That window is acceptable here because the example app is re-seedable and its - * data set is small. An app where it is not should do a proper expand/backfill/ - * contract rollout instead: ship the writes, run the backfill to completion, - * and only then switch the reads over. + * Backfill owners while public reads still use projects and projectMembers. + * The companion migration pages memberships separately, so large projects do + * not truncate the member half. Complete both migrations on each deployment + * before activating the projectAccess readers in PR #452. */ export const migration = defineMigration({ id: '20260906_022807_backfill_project_access', diff --git a/example/convex/functions/projects.ts b/example/convex/functions/projects.ts index 1f3287659..3bcc8d1dd 100644 --- a/example/convex/functions/projects.ts +++ b/example/convex/functions/projects.ts @@ -5,12 +5,10 @@ import { authMutation, authQuery, optionalAuthQuery } from '../lib/crpc'; import type { Insert, Select } from '../shared/api'; import { grantProjectAccess, - hasAnyProject, - listAccessibleProjects, - listProjectsForDropdown, revokeProjectAccess, syncProjectArchived, } from './_helpers/project_access'; +import { hasAnyProject } from './_helpers/project_existence'; import { projectMembersTable, projectsTable } from './schema'; // Schema for project list items @@ -160,15 +158,34 @@ export const list = optionalAuthQuery }; } - // One index walk over the viewer's own access rows. The previous read model - // scanned `projects` and post-filtered owner-or-member in JS, so it cost - // O(table) per page and every scanned row joined this subscription. - const results = await listAccessibleProjects(ctx, { - userId, - archived: input.includeArchived === true, - cursor: input.cursor, - limit: input.limit, + const memberships = await ctx.orm.query.projectMembers.findMany({ + where: { userId }, + limit: 1000, + columns: { projectId: true }, }); + const memberProjectIds = new Set(memberships.map((m) => m.projectId)); + + const results = await ctx.orm.query.projects + .select() + .orderBy({ createdAt: 'desc' }) + .filter(async (project: ProjectRow) => { + const isOwner = project.ownerId === userId; + const isMember = memberProjectIds.has(project.id); + + if (!(isOwner || isMember)) { + return false; + } + + if (input.includeArchived) { + return project.archived; + } + + return !project.archived; + }) + .paginate({ + cursor: input.cursor, + limit: input.limit, + }); const projectIds = results.page.map((p) => p.id); const statsByProject = await getProjectStats(projectIds); @@ -599,9 +616,45 @@ export const listForDropdown = authQuery }) ) ) - .query(({ ctx }) => { - // 1000 owned + 1000 member, the capacity the two-query read had. - return listProjectsForDropdown(ctx, { userId: ctx.userId, limit: 2000 }); + .query(async ({ ctx }) => { + const userId = ctx.userId; + + const owned = await ctx.orm.query.projects.findMany({ + where: { ownerId: userId, archived: false }, + limit: 1000, + columns: { id: true, name: true }, + extras: { isOwner: true }, + }); + + const memberRows = await ctx.orm.query.projectMembers.findMany({ + where: { userId }, + limit: 1000, + columns: { projectId: true }, + }); + + const memberProjectIds = Array.from( + new Set(memberRows.map((row) => row.projectId)) + ); + + const memberProjects = memberProjectIds.length + ? await ctx.orm.query.projects.findMany({ + where: { id: { in: memberProjectIds }, archived: false }, + limit: memberProjectIds.length, + columns: { id: true, name: true }, + extras: { isOwner: false }, + }) + : []; + + const byId = new Map< + string, + { id: string; isOwner: boolean; name: string } + >(); + for (const project of memberProjects) byId.set(project.id, project); + for (const project of owned) byId.set(project.id, project); + + return Array.from(byId.values()).sort((a, b) => + a.name.localeCompare(b.name) + ); }); export const hasAny = authQuery.output(z.boolean()).query(({ ctx }) => { diff --git a/example/convex/functions/schema.ts b/example/convex/functions/schema.ts index 80a497f03..c44920d03 100644 --- a/example/convex/functions/schema.ts +++ b/example/convex/functions/schema.ts @@ -396,10 +396,8 @@ export const projectAccessTable = convexTable( * field. A `timestamp()` here would be redundant: the value is only ever * compared, never read back as a date. * - * Millisecond resolution. `_creationTime` carries a sub-millisecond - * fraction, but it reaches this column through a hydrated `Date`, which - * cannot hold it, so two projects created in the same millisecond share a - * key and are separated by the access row's own creation order. + * Preserve raw `_creationTime`, including its fractional milliseconds. + * Hydrated Dates would reorder same-millisecond projects after backfill. */ projectCreatedAt: integer().notNull(), /** Mirrors `projects.archived` so the toggle is part of the index range. */ diff --git a/fixtures/next-auth/package.json b/fixtures/next-auth/package.json index afaa9a86a..911092375 100644 --- a/fixtures/next-auth/package.json +++ b/fixtures/next-auth/package.json @@ -9,7 +9,7 @@ "convex": "1.46.0", "hono": "4.12.9", "kitcn": "workspace:*", - "lucide-react": "^1.56.0", + "lucide-react": "^1.57.0", "next": "16.4.0", "next-themes": "^0.4.6", "react": "19.3.0", diff --git a/fixtures/next/package.json b/fixtures/next/package.json index 53fa8eec2..a6f377a1d 100644 --- a/fixtures/next/package.json +++ b/fixtures/next/package.json @@ -8,7 +8,7 @@ "convex": "1.46.0", "hono": "4.12.9", "kitcn": "workspace:*", - "lucide-react": "^1.56.0", + "lucide-react": "^1.57.0", "next": "16.4.0", "next-themes": "^0.4.6", "react": "19.3.0", diff --git a/fixtures/start-auth/package.json b/fixtures/start-auth/package.json index 7f4232299..a18bec811 100644 --- a/fixtures/start-auth/package.json +++ b/fixtures/start-auth/package.json @@ -15,7 +15,7 @@ "convex": "1.46.0", "hono": "4.12.9", "kitcn": "workspace:*", - "lucide-react": "^1.56.0", + "lucide-react": "^1.57.0", "react": "^19.3.0", "react-dom": "^19.3.0", "shadcn": "latest", diff --git a/fixtures/start/package.json b/fixtures/start/package.json index 5b2a4ff36..6bc90c624 100644 --- a/fixtures/start/package.json +++ b/fixtures/start/package.json @@ -14,7 +14,7 @@ "convex": "1.46.0", "hono": "4.12.9", "kitcn": "workspace:*", - "lucide-react": "^1.56.0", + "lucide-react": "^1.57.0", "react": "^19.3.0", "react-dom": "^19.3.0", "shadcn": "latest", diff --git a/fixtures/vite-auth/package.json b/fixtures/vite-auth/package.json index 01f7ca792..fc1a787f4 100644 --- a/fixtures/vite-auth/package.json +++ b/fixtures/vite-auth/package.json @@ -11,7 +11,7 @@ "convex": "1.46.0", "hono": "4.12.9", "kitcn": "workspace:*", - "lucide-react": "^1.56.0", + "lucide-react": "^1.57.0", "react": "^19.3.0", "react-dom": "^19.3.0", "shadcn": "latest", diff --git a/fixtures/vite/package.json b/fixtures/vite/package.json index 4847b6edf..9800a8d14 100644 --- a/fixtures/vite/package.json +++ b/fixtures/vite/package.json @@ -10,7 +10,7 @@ "convex": "1.46.0", "hono": "4.12.9", "kitcn": "workspace:*", - "lucide-react": "^1.56.0", + "lucide-react": "^1.57.0", "react": "^19.3.0", "react-dom": "^19.3.0", "shadcn": "latest", From b555b70406ffb79e486d24e50d36b596f805e6b1 Mon Sep 17 00:00:00 2001 From: zbeyens Date: Wed, 14 Oct 2026 15:05:28 +0200 Subject: [PATCH 7/7] Record preparation PR ownership and proof --- ...026-09-05-443-projects-list-read-bounds.md | 2 + docs/plans/443-project-access-preparation.md | 171 +++++++++--------- 2 files changed, 91 insertions(+), 82 deletions(-) diff --git a/docs/plans/2026-09-05-443-projects-list-read-bounds.md b/docs/plans/2026-09-05-443-projects-list-read-bounds.md index 60578e7ae..df2f134bc 100644 --- a/docs/plans/2026-09-05-443-projects-list-read-bounds.md +++ b/docs/plans/2026-09-05-443-projects-list-read-bounds.md @@ -634,3 +634,5 @@ Live feedback ledger (all raw surfaces, including resolved/outdated): | https://github.com/udecode/kitcn/pull/452#discussion_r3951347790 | P2 | Accepted owner-preserving rollback; replay source proof | | https://github.com/udecode/kitcn/pull/452#discussion_r3942887899 | P2 | Fixed source: dropdown hydrates bounded batches and returns 750 owned +750 shared; post-push replay required | | https://github.com/udecode/kitcn/pull/452#discussion_r3951352697 | P2 | Prior 2000 limit alone was insufficient; current batch hydration closes actual fanout failure | + +Preparation source is https://github.com/udecode/kitcn/pull/487, dedicated plan docs/plans/443-project-access-preparation.md. PR #452 remains the activation owner and cannot become ready before per-target preparation/backfill receipts. diff --git a/docs/plans/443-project-access-preparation.md b/docs/plans/443-project-access-preparation.md index cbd34bee7..c3cd24e1b 100644 --- a/docs/plans/443-project-access-preparation.md +++ b/docs/plans/443-project-access-preparation.md @@ -74,10 +74,10 @@ Blocked condition: Task state: - task_type: bug / staged migration preparation - task_complexity: non-trivial -- current_phase: delivery -- current_phase_status: in_progress -- next_phase: commit, final review and PR identity -- goal_status: active +- current_phase: source delivery complete +- current_phase_status: done +- next_phase: operator preparation deployment; dependent activation +- goal_status: complete preparation source slice Current verdict: - verdict: source verified; delivery in progress @@ -122,7 +122,7 @@ Start Gates: | Skill analysis before edits | yes | task/autoclosure/autogoal, architect, testing, deslop and autoreview owners loaded | | Active goal checked or created | yes | existing open-PR batch goal; no duplicate goal | | Source of truth read before edits | yes | VISION, docs owner map, issue443 and all PR452 raw feedback | -| Exact per-PR task ownership | yes | not-yet-created preparation PR slice only; separate #452 activation plan | +| Exact per-PR task ownership | yes | PR #487 only; separate #452 activation plan | | GitHub comments and attachments read | yes | 452-feedback-pre-review.json; 2 comments,9 reviews,6 threads,all12 thread comments | | Video transcript evidence required | no | N/A: no videos | | Pre-solution issue challenge required | yes | deploy-before-migrate source confirmed; alternative candidates evaluated | @@ -136,121 +136,121 @@ Start Gates: | Browser tool decision for browser surface | no | N/A: no rendered change; public endpoint integration owns proof | | Commit / PR expectation decision | yes | whole checkout normal commit/push/new preparation PR authorized by user split and task | | Task-style PR body decision | yes | emoji task body with this plan and dependent #452 | -| Task-plan PR body evidence | yes | N/A before new PR exists; final readback required | +| Task-plan PR body evidence | yes | PR487 body names this exact plan; final-head receipt verifies ownership after identity repair | | GitHub issue sync expectation decision | yes | link dependency, no fixes443 claim in preparation | | Output budget strategy recorded | yes | bounded outputs; full logs under /tmp/kitcn-pr-closure-20261014 | Work Checklist: -- [ ] If a duration was requested, it is recorded as minimum active work unless +- [x] If a duration was requested, it is recorded as minimum active work unless explicitly marked hard stop; when no better metric exists, initial and final confidence scores are recorded. -- [ ] Objective includes outcome, completion threshold, verification surface, +- [x] Objective includes outcome, completion threshold, verification surface, constraints, boundaries, and blocked condition. -- [ ] Task source classified with source type, id/link, title, task type, +- [x] Task source classified with source type, id/link, title, task type, acceptance criteria, caveats, likely files/routes/packages, browser surface, and root-cause layer. -- [ ] Every GitHub PR in scope has its own task plan. This plan owns one exact +- [x] Every GitHub PR in scope has its own task plan. This plan owns one exact PR, owns a not-yet-created PR slice, or records N/A because no PR is in scope; a batch plan is not used as a substitute. -- [ ] Required video or screen-recording evidence is cached/read as normalized +- [x] Required video or screen-recording evidence is cached/read as normalized `` XML, or marked N/A with reason. -- [ ] For public GitHub bug reports, behavior claims, technical diagnoses, or +- [x] For public GitHub bug reports, behavior claims, technical diagnoses, or suggested fixes, reporter claims are challenged before implementation with a recorded verdict: `valid`, `not reproduced`, `invalid`, `wont-fix`, `partially valid`, or `platform limitation`. Feature, docs, support, or cleanup requests with no bug claim may mark reproduction `N/A` with reason. -- [ ] Repro escalation ladder followed for bug/behavior claims: focused +- [x] Repro escalation ladder followed for bug/behavior claims: focused test/source-level repro first when applicable; existing repo-owned automated browser or integration proof next when available and useful as executable coverage; the repo-approved Browser tool next when tests or automation cannot reproduce or cannot model the surface honestly; screenshot or explicit visual-proof waiver when visual/native state matters. -- [ ] Hard-stop rule followed for bug/behavior claims: no code when the issue +- [x] Hard-stop rule followed for bug/behavior claims: no code when the issue is not reproduced, invalid, or won't-fix; partial validity pivots to the best long-term fix and records what was wrong or incomplete in the issue's proposed path. -- [ ] Nearby repo instructions and implementation patterns read before edits. -- [ ] Source-listed case matrix is complete and every contradiction has an +- [x] Nearby repo instructions and implementation patterns read before edits. +- [x] Source-listed case matrix is complete and every contradiction has an owner, harness, and verdict before mutation. -- [ ] Readiness is classified `ready`, `repair-source`, `major`, `blocked`, or +- [x] Readiness is classified `ready`, `repair-source`, `major`, `blocked`, or `invalid` with evidence. -- [ ] Implementation fixes the right ownership boundary, or the narrower choice +- [x] Implementation fixes the right ownership boundary, or the narrower choice is recorded with reason. -- [ ] Release artifact requirement recorded: active changeset, new changeset, or +- [x] Release artifact requirement recorded: active changeset, new changeset, or N/A with reason. -- [ ] Final handoff shape decided: bug/feature/testing/batch/review/GitHub +- [x] Final handoff shape decided: bug/feature/testing/batch/review/GitHub requirements, PR body sync, and issue sync when applicable. -- [ ] Commit/PR handling recorded for code-changing work: commit and PR +- [x] Commit/PR handling recorded for code-changing work: commit and PR completed, no local patch, user explicitly declined, or blocker recorded. "User did not separately ask for a PR" is not a valid blocker. -- [ ] PR body shape recorded: PR #270 emoji task-style body used, N/A reason +- [x] PR body shape recorded: PR #270 emoji task-style body used, N/A reason recorded, or blocker recorded. -- [ ] PR task evidence recorded: body includes `๐Ÿงญ Task plan: ...`, the plan +- [x] PR task evidence recorded: body includes `๐Ÿงญ Task plan: ...`, the plan exists at the PR head, and it identifies the exact PR before autoclosure. -- [ ] Branch handling recorded for code-changing work: dedicated branch used, +- [x] Branch handling recorded for code-changing work: dedicated branch used, new branch needed, or N/A with reason. -- [ ] Local-env-rot retry policy recorded for any surprising repo-wide failure: +- [x] Local-env-rot retry policy recorded for any surprising repo-wide failure: reinstall/rerun evidence or N/A with reason. -- [ ] Workspace authority recorded: every proof command names the cwd/tool that +- [x] Workspace authority recorded: every proof command names the cwd/tool that owns the changed behavior. -- [ ] Output budget discipline recorded and followed: broad searches are +- [x] Output budget discipline recorded and followed: broad searches are scoped, capped, counted, or artifacted instead of streamed into goal context. -- [ ] High-risk note recorded for public API, runtime, package-boundary, +- [x] High-risk note recorded for public API, runtime, package-boundary, browser behavior, agent-action, or command-contract changes, or marked N/A with reason. -- [ ] Review/autoreview target selected from actual diff state for non-trivial +- [x] Review/autoreview target selected from actual diff state for non-trivial implementation work, or marked N/A with reason. -- [ ] Agent-native review decision recorded for `.agents/**`, `.claude/**`, +- [x] Agent-native review decision recorded for `.agents/**`, `.claude/**`, `.codex/**`, skills, hooks, commands, prompts, or user-action tooling. Completion Gates: | Gate | Applies | Required action | Evidence | |------|---------|-----------------|----------| -| Named verification threshold | pending | Run the command, proof, source audit, or artifact check named in this plan | pending | -| Exact per-PR task ownership | pending | Record the exact PR and dedicated plan, or the not-yet-created single-PR slice | pending | -| Pre-solution issue challenge verdict | pending | Record reporter claim, suggested fix, repro verdict, validity verdict, durable boundary, and hard-stop/pivot decision before implementation | pending | -| Repro escalation ladder | pending | For bug/behavior claims, record test/source-level, automated browser/integration, Browser, and screenshot/visual-proof outcomes or N/A/blocker reasons before `not reproduced` | pending | -| Bug reproduced before fix | pending | Record failing test/repro or N/A with reason | pending | -| Targeted behavior verification | pending | Run focused test/proof for changed behavior or record N/A | pending | -| TypeScript or typed config changed | pending | Run relevant typecheck | pending | -| Package exports or file layout changed | pending | Run the relevant package build before final verification and keep generated updates | pending | -| Package manifests, lockfile, or install graph changed | pending | Run `bun install` and relevant package checks | pending | -| Agent rules or skills changed | pending | Run `bun install` and verify generated skill sync | pending | -| Workspace authority proof | pending | Run verification in the owning repo/package/app/route/tool and record cwd; do not count the wrong workspace as proof | pending | -| Browser surface changed | pending | Capture Browser Use proof or record explicit waiver/blocker | pending | -| Browser final proof | pending | Attach screenshot or exact browser verification caveat when browser proof applies | pending | -| UI walkthrough | pending | If UI or rendered output changed, run `.agents/skills/walkthrough/SKILL.md` after final proof and show annotated images in the final handoff; otherwise record N/A | pending | -| Scaffold or fixture output changed | pending | Run `bun run fixtures:sync` and `bun run fixtures:check`, or record N/A | pending | -| Package behavior or public API changed | pending | Add a changeset or record why no changeset applies | pending | -| Docs and kitcn skill sync changed | pending | Keep `www/**` and `packages/kitcn/skills/kitcn/**` in sync, or record N/A | pending | -| Docs or content changed | pending | For docs-heavy work, use `--template docs`; for incidental docs, verify source-backed claims, links, examples, and rendered output or record N/A | pending | -| High-risk mini gate | pending | For public API/runtime/package-boundary/browser/agent-action/command-contract changes, record realistic failure mode, proof plan, and why the chosen boundary is right; otherwise N/A | pending | -| Agent-native review for agent/tooling changes | pending | For `.agents/**`, `.claude/**`, `.codex/**`, skills, hooks, commands, prompts, or user-action tooling, load `.agents/skills/agent-native-reviewer/SKILL.md` and close accepted/actionable findings, or record N/A | pending | -| Local install corruption suspected | pending | Run `bun install` once, rerun the exact failing command, or record N/A | pending | -| Commit created | pending | For verified code-changing work, stage the entire current checkout per repo policy and create a commit; N/A only for no local patch, explicit user decline, analytical/blocked/inconclusive work, or recorded external blocker | pending | -| PR create or update | pending | For verified code-changing work, run `check`, push, create or update the PR, and sync PR body to the task-style final handoff; N/A only for no local patch, explicit user decline, analytical/blocked/inconclusive work, or recorded external blocker | pending | -| Task-style PR body verified | pending | Verify the PR body with `gh pr view --json body`; it must preserve auto-release blocks when applicable, must not include a current-PR self-link, and must use the PR #270 emoji format: `๐Ÿ› Fixes ...`, `๐ŸŸข 95-100% confidence`, `Phase / ๐Ÿงช Tests / ๐ŸŒ Browser` table, and bold emoji Outcome/Caveat/Design/Verified sections | pending | -| PR task evidence verified | pending | Verify body plan line, plan at PR head, and exact PR ownership | pending | -| PR proof image hosting | pending | If PR body needs browser proof, replace local image paths with hosted GitHub URLs or record N/A | pending | -| GitHub issue sync-back | pending | Post concise issue sync after PR exists, or record N/A/blocker | pending | -| Final handoff contract | pending | Fill the final handoff fields below with exact PR/issue/confidence/tests/browser/outcome/caveats/design/verification content or N/A reason | pending | -| Final lint | pending | Run `bun lint:fix` or scoped equivalent | pending | -| Output budget discipline | pending | Verify no unbounded high-volume command output was streamed, or record the accidental output and recovery | pending | -| Timed checkpoint | pending | If duration was requested, keep improving until elapsed, then finish the current loop cleanly; otherwise N/A | pending | -| Autoreview for non-trivial implementation changes | pending | Load `.agents/skills/autoreview/SKILL.md`; use dirty local `--mode local`, branch/PR `--mode branch --base `, or committed slice `--mode commit --commit ` until no accepted/actionable findings, or record N/A for docs-only/trivial/no local patch | pending | -| Goal plan complete | yes | Run `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/443-project-access-preparation.md` | pending | +| Named verification threshold | yes | Run the command, proof, source audit, or artifact check named in this plan | Focused24 and full bun check passed including fixture/verify/runtime lanes | +| Exact per-PR task ownership | yes | Record the exact PR and dedicated plan, or the not-yet-created single-PR slice | PR487 body plan path readback; this dedicated plan identifies exactPR487; final-head receipt validates all OIDs | +| Pre-solution issue challenge verdict | yes | Record reporter claim, suggested fix, repro verdict, validity verdict, durable boundary, and hard-stop/pivot decision before implementation | Valid deploy-before-migrate defect; staged revision boundary approved by user | +| Repro escalation ladder | yes | For bug/behavior claims, record test/source-level, automated browser/integration, Browser, and screenshot/visual-proof outcomes or N/A/blocker reasons before `not reproduced` | Source path + real authenticated endpoint tests; browser N/A no rendered change | +| Bug reproduced before fix | yes | Record failing test/repro or N/A with reason | Fractional order and1500-project dropdown red logs; actual missing-wiring red probes | +| Targeted behavior verification | yes | Run focused test/proof for changed behavior or record N/A | Four focused suites24 tests passed; post-push replay recorded externally | +| TypeScript or typed config changed | yes | Run relevant typecheck | Full bun check typecheck passed | +| Package exports or file layout changed | no | Run the relevant package build before final verification and keep generated updates | N/A: no published source/export change; package build ran only for codegen | +| Package manifests, lockfile, or install graph changed | yes | Run `bun install` and relevant package checks | Generated fixture package pins converged through fixtures:sync; all fresh fixture installs/checks passed | +| Agent rules or skills changed | no | Run `bun install` and verify generated skill sync | N/A: none changed | +| Workspace authority proof | yes | Run verification in the owning repo/package/app/route/tool and record cwd; do not count the wrong workspace as proof | All source/check commands ran in /Users/zbeyens/git/better-convex; isolated Convex codegen owns generated artifacts | +| Browser surface changed | no | Capture Browser Use proof or record explicit waiver/blocker | N/A: canonical public readers retained, no rendered output change | +| Browser final proof | no | Attach screenshot or exact browser verification caveat when browser proof applies | N/A: real registered endpoint integration; no visual claim | +| UI walkthrough | no | If UI or rendered output changed, run `.agents/skills/walkthrough/SKILL.md` after final proof and show annotated images in the final handoff; otherwise record N/A | N/A: no UI/rendered change | +| Scaffold or fixture output changed | yes | Run `bun run fixtures:sync` and `bun run fixtures:check`, or record N/A | fixtures:sync and all eight fixtures:check passed; six generated lucide pins retained | +| Package behavior or public API changed | no | Add a changeset or record why no changeset applies | N/A: no package behavior/API change; example-private storage only | +| Docs and kitcn skill sync changed | no | Keep `www/**` and `packages/kitcn/skills/kitcn/**` in sync, or record N/A | N/A: no www or published user skill changes | +| Docs or content changed | yes | For docs-heavy work, use `--template docs`; for incidental docs, verify source-backed claims, links, examples, and rendered output or record N/A | Dedicated plans preserve task ownership, user-approved staged scope and archive deferral; links/source paths checked | +| High-risk mini gate | yes | For public API/runtime/package-boundary/browser/agent-action/command-contract changes, record realistic failure mode, proof plan, and why the chosen boundary is right; otherwise N/A | Empty access table cannot activate derived reads in preparation; actual registered public owner-list/member-dropdown/hasAny test | +| Agent-native review for agent/tooling changes | yes | For `.agents/**`, `.claude/**`, `.codex/**`, skills, hooks, commands, prompts, or user-action tooling, load `.agents/skills/agent-native-reviewer/SKILL.md` and close accepted/actionable findings, or record N/A | Existing deployment/status CLI route audited; no changed agent integration; operator target/approval boundary explicit | +| Local install corruption suspected | no | Run `bun install` once, rerun the exact failing command, or record N/A | N/A: no install corruption; isolated setup missing dependencies corrected at owning example resolution | +| Commit created | yes | For verified code-changing work, stage the entire current checkout per repo policy and create a commit; N/A only for no local patch, explicit user decline, analytical/blocked/inconclusive work, or recorded external blocker | Whole checkout commit df20cb263d23314d449c5424bf467dfb61f2cf45; identity closeout follows normal push | +| PR create or update | yes | For verified code-changing work, run `check`, push, create or update the PR, and sync PR body to the task-style final handoff; N/A only for no local patch, explicit user decline, analytical/blocked/inconclusive work, or recorded external blocker | PR487 created after full bun check and clean committed autoreview; normal branch push | +| Task-style PR body verified | yes | Verify the PR body with `gh pr view --json body`; it must preserve auto-release blocks when applicable, must not include a current-PR self-link, and must use the PR #270 emoji format: `๐Ÿ› Fixes ...`, `๐ŸŸข 95-100% confidence`, `Phase / ๐Ÿงช Tests / ๐ŸŒ Browser` table, and bold emoji Outcome/Caveat/Design/Verified sections | 487-created-readback.json confirms emoji task body, dedicated plan, caveats and tests | +| PR task evidence verified | yes | Verify body plan line, plan at PR head, and exact PR ownership | PR487 body has one plan line; this plan repairs exact487 identity; immutable-head receipt validates final push | +| PR proof image hosting | no | If PR body needs browser proof, replace local image paths with hosted GitHub URLs or record N/A | N/A: no visual proof required | +| GitHub issue sync-back | no | Post concise issue sync after PR exists, or record N/A/blocker | N/A: preparation is supporting slice; PR body links443 and dependent452 without claiming issue closure | +| Final handoff contract | yes | Fill the final handoff fields below with exact PR/issue/confidence/tests/browser/outcome/caveats/design/verification content or N/A reason | PR487/source proof/preparation caveat recorded below; final external receipt owns head identity | +| Final lint | yes | Run `bun lint:fix` or scoped equivalent | bun lint:fix and full check lint passed | +| Output budget discipline | yes | Verify no unbounded high-volume command output was streamed, or record the accidental output and recovery | Bounded text; full logs in /tmp/kitcn-pr-closure-20261014; known isolated setup errors recorded | +| Timed checkpoint | no | If duration was requested, keep improving until elapsed, then finish the current loop cleanly; otherwise N/A | N/A: no timed request | +| Autoreview for non-trivial implementation changes | yes | Load `.agents/skills/autoreview/SKILL.md`; use dirty local `--mode local`, branch/PR `--mode branch --base `, or committed slice `--mode commit --commit ` until no accepted/actionable findings, or record N/A for docs-only/trivial/no local patch | Frozen committed branch df20cb263d23314d449c5424bf467dfb61f2cf45 clean; initial mutable review failed closed and is not counted | +| Goal plan complete | yes | Run `node .agents/skills/autogoal/scripts/check-complete.mjs docs/plans/443-project-access-preparation.md` | check-complete.mjs runs on this materialized plan before final push; final receipt owns live external proof | Phase / pass table: | Phase | Status | Evidence | Next | -|-------|--------|----------|------| -| Intake and source read | in_progress | created plan | implementation | -| Implementation | pending | | verification | -| Verification | pending | | closeout | -| Commit / PR / GitHub sync | pending | | final response | -| Closeout | pending | | final response | +| --- | --- | --- | --- | +| Intake | done | user-approved split and source review | proof | +| Implementation | done | preparation retains canonical reads; maintained access and paged migrations | proof | +| Verification | done | focused24 and full check incl runtime passed | delivery | +| Delivery | done | PR487 created after clean review; plan identity closeout normal push | external receipt | +| Closeout | done | source slice complete; activation separately gated | operator deployment | Findings: - See rollout contract and verification evidence below. @@ -272,7 +272,7 @@ Error attempts: Verification evidence: - Focused24 tests in four suites passed. Full bun check passed on2026-10-14, including lint, typecheck, all tests, eight fixture checks, verify and runtime scenarios. Logs /tmp/kitcn-pr-closure-20261014/preparation-check.log and preparation-focused.log. - Source codegen ran on isolated anonymous Convex ports3235/3236; generated API/data model and procedure-name locations retained. Fixtures regenerated through CLI, six lucide snapshot pins converged. -- Three independent cleanup lenses completed; only accepted P3 deletions applied. Frozen committed-branch autoreview remains before push. +- Three independent cleanup lenses completed; only accepted P3 deletions applied. Frozen committed-branch autoreview on df20cb263d23314d449c5424bf467dfb61f2cf45 passed with no accepted/actionable findings. Source-listed case matrix: | Case | Source claim | Harness | Before | Expected after | Evidence | Status | @@ -284,10 +284,10 @@ Source-listed case matrix: | Dropdown capacity | P2 | 750 owned + 750 shared | relation fanout exception | 1500 unique rows | access suite | passed | Final handoff contract: -- Commit line: pending -- PR line: pending -- Issue line: pending -- Confidence line: pending +- Commit line: df20cb263d23314d449c5424bf467dfb61f2cf45 plus plan identity closeout +- PR line: https://github.com/udecode/kitcn/pull/487 +- Issue line: supports #443; activation #452 owns completion +- Confidence line: 95-100% preparation source; no activation claim - Flow table: - Reproduced: red fractional timestamp/dropdown/lifecycle wiring probes; browser N/A - Verified: focused suites passed; final check in progress; browser N/A @@ -298,8 +298,8 @@ Final handoff contract: - Chosen boundary: deployable preparation and activation revisions - Why not quick patch: same-head deployment activates reads before migration completion - Why not broader change: no compatibility reader, ORM union redesign or new archive contract -- Verified: pending -- PR body verified: pending +- Verified: focused24, full bun check, independent lenses, clean committed autoreview, real generated codegen +- PR body verified: gh pr view487 body readback names dedicated plan and retained-read contract Task-style PR body contract: - Preserve any existing `` block. If a changeset is @@ -323,9 +323,9 @@ Task-style PR body contract: of that output. Final handoff / sync: -- Commit: pending -- PR: pending -- Issue: pending +- Commit: df20cb263d23314d449c5424bf467dfb61f2cf45 plus identity closeout +- PR: https://github.com/udecode/kitcn/pull/487 +- Issue: supports443; does not close it - Browser proof: N/A: no rendered output change - Caveats: activation receipts absent; no remote deployment performed @@ -335,8 +335,8 @@ Timeline: Reboot status: | Question | Answer | |----------|--------| -| Where am I? | Intake and source read | -| Where am I going? | Implementation, verification, commit/PR/GitHub sync, closeout | +| Where am I? | Preparation source delivered as PR487 | +| Where am I going? | Operator preparation deployment/backfills; activation452 remains gated | | What is the goal? | Prepare access while retaining canonical reads | | What have I learned? | See Findings | | What have I done? | See Timeline | @@ -351,7 +351,7 @@ Hard closeout guard: Authorized requirements and scope baseline: - User requested review and cleanup pushes for all open PRs; explicitly approved separate preparation/activation PRs and deferral of the archive >10,000 access-row P2. -- This plan owns only the not-yet-created preparation PR. PR #452 has its own activation plan. +- This plan owns exactly https://github.com/udecode/kitcn/pull/487. PR #452 has its own activation plan. - Preserve all canonical public reads until completed preparation deployment/backfills. No force push, merge or real remote deploy authorized. - Live archive finding remains open at https://github.com/udecode/kitcn/pull/452#discussion_r3942887895 with explicit user deferral. - Package source/API unchanged; no changeset required. Build package only to run codegen. Generated Convex files come from an isolated anonymous local backend, never hand edits. @@ -376,3 +376,10 @@ Operator rollout contract (not executed): 3. Only after every target has those receipts, authorize activation #452. A clean test run or two commits in one PR does not substitute. - Agent-native parity audit: existing CLI deployment and migration-status commands expose each operation and its JSON status. No new helper, hook, skill or agent integration introduced. Deployment target/approval remains an explicit operator boundary. - Prove It Works drove actual authenticated handler and real local codegen proof. Fix Root Causes drove the staged revision boundary rather than a readiness flag. Model the Domain keeps canonical entitlement writes in one helper and deletion in FK cascades. + +Live preparation feedback ledger: +| URL | Priority | Verdict | +| --- | --- | --- | +| https://github.com/udecode/kitcn/pull/487#issuecomment-6168988902 | P3 | Status only: preview building; no source finding | +| https://github.com/udecode/kitcn/pull/487#issuecomment-6168991505 | P3 | Quota notice, no source finding; independent local review completed | +| https://github.com/udecode/kitcn/pull/487#issuecomment-6168992850 | P3 | No changeset expected: private example/test-only source changes |